![]() |
|
Message-ID: <CAN30aBG+7j6Ki3Q3s-0M7CFFa_XFw4cdg7HuOr5B=_FgF_qbDg@mail.gmail.com>
Date: Sun, 9 Sep 2018 12:04:32 -0700
From: Fāng-ruì Sòng <emacsray@...il.com>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] simplify __procfdname by folding the 0 case
On Sun, Sep 2, 2018 at 4:15 AM Alexander Monakov <amonakov@...ras.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
>
Content of type "text/html" skipped
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.