Re: [PATCH] simplify __procfdname by folding the 0 case

["Fāng-ruì Sòng" <emacsray@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]

On Sun, Sep 2, 2018 at 4:15 AM Alexander Monakov <amonakov@ispras.ru> wrote:

> In 2016 I sent a more comprehensive cleanup, please review the thread
> starting at http://openwall.com/lists/musl/2016/02/21/2
>
> Some notes on the patch below.
>
> On Sun, 2 Sep 2018, Fangrui Song wrote:
> > --- a/src/internal/procfdname.c
> > +++ b/src/internal/procfdname.c
> > @@ -2,12 +2,7 @@ void __procfdname(char *buf, unsigned fd)
> >  {
> >       unsigned i, j;
> >       for (i=0; (buf[i] = "/proc/self/fd/"[i]); i++);
> > -     if (!fd) {
> > -             buf[i] = '0';
> > -             buf[i+1] = 0;
> > -             return;
> > -     }
> > -     for (j=fd; j; j/=10, i++);
> > -     buf[i] = 0;
> > -     for (; fd; fd/=10) buf[--i] = '0' + fd%10;
> > +     for (j=fd; i++, j /= 10; );
>
> This is not correct as it only increments i once. A do-while loop would do
> the
> job better here.
>

May I defend for myself?  for (j=fd; i++, j /= 10; );
i++ is in the loop condition so it will be incremented multiple times.


> > +     buf[i] = '\0';
> > +     while (buf[--i] = '0' + fd%10, fd /= 10);
>
> Likewise, a do-while loop would be more suitable here.
>

I'm still learning musl's code style so may likely make the code look ugly
:)


> >  }
>
> Alexander
>

[-- Attachment #2: Type: text/html, Size: 2160 bytes --]