* [PATCH] simplify __procfdname by folding the 0 case
@ 2018-09-02 7:51 Fangrui Song
2018-09-02 11:14 ` Alexander Monakov
0 siblings, 1 reply; 4+ messages in thread
From: Fangrui Song @ 2018-09-02 7:51 UTC (permalink / raw)
To: musl
---
src/internal/procfdname.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/src/internal/procfdname.c b/src/internal/procfdname.c
index 697e0bdc..5046abaa 100644
--- 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; );
+ buf[i] = '\0';
+ while (buf[--i] = '0' + fd%10, fd /= 10);
}
--
2.18.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] simplify __procfdname by folding the 0 case
2018-09-02 7:51 [PATCH] simplify __procfdname by folding the 0 case Fangrui Song
@ 2018-09-02 11:14 ` Alexander Monakov
2018-09-09 19:04 ` Fāng-ruì Sòng
0 siblings, 1 reply; 4+ messages in thread
From: Alexander Monakov @ 2018-09-02 11:14 UTC (permalink / raw)
To: musl
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.
> + buf[i] = '\0';
> + while (buf[--i] = '0' + fd%10, fd /= 10);
Likewise, a do-while loop would be more suitable here.
> }
Alexander
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] simplify __procfdname by folding the 0 case
2018-09-02 11:14 ` Alexander Monakov
@ 2018-09-09 19:04 ` Fāng-ruì Sòng
2018-09-09 21:30 ` Alexander Monakov
0 siblings, 1 reply; 4+ messages in thread
From: Fāng-ruì Sòng @ 2018-09-09 19:04 UTC (permalink / raw)
To: musl
[-- 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 --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-09-09 21:30 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-02 7:51 [PATCH] simplify __procfdname by folding the 0 case Fangrui Song
2018-09-02 11:14 ` Alexander Monakov
2018-09-09 19:04 ` Fāng-ruì Sòng
2018-09-09 21:30 ` Alexander Monakov
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).