From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/13217 Path: news.gmane.org!.POSTED!not-for-mail From: =?UTF-8?B?RsSBbmctcnXDrCBTw7JuZw==?= Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] simplify __procfdname by folding the 0 case Date: Sun, 9 Sep 2018 12:04:32 -0700 Message-ID: References: <5b8b9a8b.1c69fb81.ed159.bb37@mx.google.com> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="000000000000ce841d057574e9c2" X-Trace: blaine.gmane.org 1536519771 13289 195.159.176.226 (9 Sep 2018 19:02:51 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sun, 9 Sep 2018 19:02:51 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-13233-gllmg-musl=m.gmane.org@lists.openwall.com Sun Sep 09 21:02:47 2018 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1fz4ys-0003KW-U1 for gllmg-musl@m.gmane.org; Sun, 09 Sep 2018 21:02:47 +0200 Original-Received: (qmail 1542 invoked by uid 550); 9 Sep 2018 19:04:55 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 1521 invoked from network); 9 Sep 2018 19:04:54 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=UtcS/BOzoCaF+5CT8TAnhBFyzxqsXoISYi/++fIW+cA=; b=GkzuEPti4UeHkMF8Eji/LVrq8eN4kOEnpyQMW7kICEhk7xMXixQYLrQh1tEFV08OJI a9sLoES01fQnDEQ72/IvhhQ1kRKDvki/uklAkK0pm3biV678SLvj2y8FZ+wmlepfQvrc 5iyXrk04B37b7SmwJ9ndKfjINQ7FDhwHOpKOxAZkXZr8JBJdxkFyUhcG5prgx+BkFoYI apaSBcgqgV0eBwgM/2Rd+oF7czKJSTj5t8nvEUwE9qAjuHE2RG4FkEY5yvdedxf0ujrh 6GYNa1fVwPFfOl9kLxNz+J+LWnwC6N4o+C2zds7l+wL5EvQzka5cgbEEm7TaSWeUghYU nfOg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=UtcS/BOzoCaF+5CT8TAnhBFyzxqsXoISYi/++fIW+cA=; b=Y1pNH/Xyug4XdDrwtNwEzFmEEaFhY+5WyP1gP4mSNig2gpozv5jy+NnVptkO5c9xkA Tt3mVuGgWKdzTpLvYg3pJWfYgye0IsvCiutg11cklQvxFDVbYn6tNR4a2BqyPV9MW1Ms eReQwQXdUVk1qDy/65xXqPLB/ZSXEd5vm3l/KzmdPLcpJjLnF5JDsXI2PzwNcV6xs35c 8kYsSbGc8IgC1azjPvSN4kK0kfgeqCN24Ks9WuHBaVs+n2U1kpURwn6J+Uiol50BgiqF z7Me66QEjHc/OMgxz/Epa/YevLSWNRI8xcGebrOgGKmhllaZ30vM9au2b3QId2Nly7dh 227Q== X-Gm-Message-State: APzg51DVm+75DnSrV8VmirP8Y/uMQMUHRof0akQlusDhEiu3XKfRh7jC DkASUMgooEvqYAzUKqJA8SJKHobcb6AfWRaoJ2Akmr2Q X-Google-Smtp-Source: ANB0VdZNRZseA5t+Z0NqtGPK4ZzFr6bFNtjj9FPkyB/jBeNg1YtAp2Dx6pp9FzoPnguZTkbdqwVSW+InJWLhel0Fafw= X-Received: by 2002:a54:4f88:: with SMTP id g8-v6mr18378413oiy.191.1536519882833; Sun, 09 Sep 2018 12:04:42 -0700 (PDT) In-Reply-To: Xref: news.gmane.org gmane.linux.lib.musl.general:13217 Archived-At: --000000000000ce841d057574e9c2 Content-Type: text/plain; charset="UTF-8" On Sun, Sep 2, 2018 at 4:15 AM Alexander Monakov 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 > --000000000000ce841d057574e9c2 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
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)
>=C2=A0 {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned i, j;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0for (i=3D0; (buf[i] =3D "/proc/self/fd/= "[i]); i++);
> -=C2=A0 =C2=A0 =C2=A0if (!fd) {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0buf[i] =3D '0'= ;;
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0buf[i+1] =3D 0;
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return;
> -=C2=A0 =C2=A0 =C2=A0}
> -=C2=A0 =C2=A0 =C2=A0for (j=3Dfd; j; j/=3D10, i++);
> -=C2=A0 =C2=A0 =C2=A0buf[i] =3D 0;
> -=C2=A0 =C2=A0 =C2=A0for (; fd; fd/=3D10) buf[--i] =3D '0' + f= d%10;
> +=C2=A0 =C2=A0 =C2=A0for (j=3Dfd; i++, j /=3D 10; );

This is not correct as it only increments i once. A do-while loop would do = the
job better here.



> +=C2=A0 =C2=A0 =C2=A0buf[i] =3D '\0';
> +=C2=A0 =C2=A0 =C2=A0while (buf[--i] =3D '0' + fd%10, fd /=3D = 10);

Likewise, a do-while loop would be more suitable here.

>=C2=A0 }

Alexander
--000000000000ce841d057574e9c2--