From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id F3ED83857829 for ; Thu, 18 Mar 2021 10:30:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org F3ED83857829 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tdevries@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 175D3AC75; Thu, 18 Mar 2021 10:30:01 +0000 (UTC) Subject: Re: [committed] Make main more readable To: Jakub Jelinek Cc: dwz@sourceware.org, mark@klomp.org References: <20210317131444.GA11885@delia> <20210317131802.GG231854@tucnak> <20210317172818.GI231854@tucnak> From: Tom de Vries Message-ID: Date: Thu, 18 Mar 2021 11:30:00 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 MIME-Version: 1.0 In-Reply-To: <20210317172818.GI231854@tucnak> Content-Type: multipart/mixed; boundary="------------7E25AF88854457C986A004FF" Content-Language: en-US X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: dwz@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Dwz mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Mar 2021 10:30:04 -0000 This is a multi-part message in MIME format. --------------7E25AF88854457C986A004FF Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit On 3/17/21 6:28 PM, Jakub Jelinek wrote: > On Wed, Mar 17, 2021 at 02:37:57PM +0100, Tom de Vries wrote: >> On 3/17/21 2:18 PM, Jakub Jelinek wrote: >>> On Wed, Mar 17, 2021 at 02:14:45PM +0100, Tom de Vries wrote: >>>> @@ -17048,10 +17050,12 @@ main (int argc, char *argv[]) >>>> outfile = NULL; >>>> hardlink = false; >>>> parse_args (argc, argv, &hardlink, &outfile); >>>> + nr_files = argc - optind; >>>> + files = (const char **)&argv[optind]; >>>> >>>> - if (optind == argc || optind + 1 == argc) >>>> + if (nr_files <= 1) >>>> { >>>> - const char *file = optind == argc ? "a.out" : argv[optind]; >>>> + const char *file = nr_files == 0 ? "a.out" : files[0]; >>> >>> Isn't that aliasing violation? >> >> Sorry, I don't see it, can you be specific about which entity is >> accessed with an incompatible type? > > char * and const char * are different types from C aliasing POV I think. > Now, as these are arguments of main and argv can be const char ** > or char ** interchangeably, what is the dynamic type is a little bit fuzzy, > but I'd say mixing accesses to argv array elements, accessing some of them > through char * effective type (e.g. in getopt_long) and others through > const char * effective type is problematic. > Making files char ** and then casting if needed (say (const char *)(files[0])) > is fine of course. I've prepared a patch implementing that suggestion. Does that address your concerns? [ FWIW, I've read through a C99 draft pdf to refresh my memory on this topic. Looking at the effective type of **argv, AFAIU it falls into the catagory "For all other accesses to an object having no declared type, the effective type of the object is simply the type of the lvalue used for the access". In other words, the effective type is char. Then I read: ... An object shall have its stored value accessed only by an lvalue expression that has one of the following types: — a qualified version of a type compatible with the effective type of the object, ... So, const char is a qualified version of a type char compatible with the effective type of the object (char). So I still don't see the problem. ] Thanks, - Tom --------------7E25AF88854457C986A004FF Content-Type: text/x-patch; charset=UTF-8; name="0001-Change-files-var-in-main-to-char.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline; filename="0001-Change-files-var-in-main-to-char.patch" Change files var in main to char ** There's a concern that the cast on &argv[optind] to const char **: =2E.. main (int argc, char *argv[]) { ... const char **files; ... files =3D (const char **)&argv[optind]; =2E.. violates the aliasing rules. Fix this by changing the type of files to char **. 2021-03-18 Tom de Vries * dwz.c (dwz): Make files param a char **. (dwz_files): Make files param char *[]. (main): Make files var a char **. --- dwz.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dwz.c b/dwz.c index 7b67960..a7aa23b 100644 --- a/dwz.c +++ b/dwz.c @@ -15267,7 +15267,7 @@ struct file_result over FILE. */ static int dwz (const char *file, const char *outfile, struct file_result *res, - struct file_result *resa, const char **files) + struct file_result *resa, char **files) { DSO *dso; int ret =3D 0, fd; @@ -16285,7 +16285,7 @@ dwz_one_file (const char *file, const char *outfi= le) /* Dwarf-compress FILES. If HARDLINK, detect if some files are hardlink= s and if so, dwarf-compress just one, and relink the others. */ static int -dwz_files (int nr_files, const char *files[], bool hardlink) +dwz_files (int nr_files, char *files[], bool hardlink) { int ret =3D 0; int i; @@ -16413,7 +16413,7 @@ main (int argc, char *argv[]) const char *outfile; bool hardlink; int nr_files; - const char **files; + char **files; =20 if (elf_version (EV_CURRENT) =3D=3D EV_NONE) error (1, 0, "library out of date"); @@ -16422,7 +16422,7 @@ main (int argc, char *argv[]) hardlink =3D false; parse_args (argc, argv, &hardlink, &outfile); nr_files =3D argc - optind; - files =3D (const char **)&argv[optind]; + files =3D &argv[optind]; =20 if (nr_files <=3D 1) { --------------7E25AF88854457C986A004FF--