[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH v4] readlink: slight size optimization
From:       Eric Blake <eblake () redhat ! com>
Date:       2023-04-25 13:09:48
Message-ID: u62q4qpkofs3xl642eoydk647n4bho7fgb4llmng6jm3mzwk3t () 6nza6ohramjb
[Download RAW message or body]

On Tue, Apr 25, 2023 at 07:51:57AM -0500, Eric Blake wrote:
> On Tue, Apr 25, 2023 at 10:52:45AM +1000, David Leonard wrote:
> > On Mon, 24 Apr 2023, Harald van Dijk wrote:
> > > coreutils/readlink.c:91:27: warning: adding 'unsigned int' to a string
> > > does not append to the string [-Wstring-plus-int]
> > >        printf("%s%s", buf, "\n" + (opt & 1));
> > >                            ~~~~~^~~~~~~~~~~
> > > coreutils/readlink.c:91:27: note: use array indexing to silence this warning
> > >        printf("%s%s", buf, "\n" + (opt & 1));
> > >                                 ^
> > >                            &    [          ]
> > > 
> > > I am not sure what busybox's intent is here. If busybox decides that
> > > this warning is useless, it would be nice if it got suppressed reliably.
> > 
> > Alternatively, avoid warnings with this construction which gives pretty
> > good size results:
> > 
> > 	if (opt)
> 
> That's not the same as 'if (opt & 1)'.  As such, you are not comparing
> apples to oranges...
> 
> >     $ cat f1.c
> >     #include <stdio.h>
> > 
> >     /* suppress trailing newline when opt */
> >     int f1(int opt, char buf[]) {
> >     #if defined(T0)
> > 	if (opt) printf("%s", buf); else printf("%s\n", buf);
> >     #elif defined(T1)
> > 	if (opt) fputs(buf, stdout); else puts(buf);
> >     #elif defined(T2)
> > 	printf("%s%s", buf, "\n" + (opt & 1));
> 
> Of the four tests, T2 is the only one with correct semantics.
> 
> >     #elif defined(T3)
> > 	printf("%s%s", buf, opt ? "" : "\n");
> >     #else
> >     # error "forgot to -DTx"
> >     #endif

The baseline in the existing coreutils/readlink.c is:

  printf((opt & 1) ? "%s" : "%s\n", buf);

which I don't see in your tests.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]