cluts comments

cluts comments

[Szabolcs Nagy]

i've reported these on irc, but luka was not there
so before i forget:

alloc.c:127: WEXITSTAUS(stat) used after !WIFEXITED(stat)

alloc.c should include signal.h (it uses sigaction)

alloc.c does not compile with glibc here
it seems _POSIX_C_SOURCE is not enough for
glibc to have SA_NODEFER in signal.h
i guess that's a glibc bug..

i've seen mixed use of %i and %d in printf
i had to look up the difference (nothing)
i recommend using only one of them consistently
(i prefer %d)

Re: cluts comments

[Solar Designer]

On Wed, Aug 03, 2011 at 07:04:07PM +0200, Szabolcs Nagy wrote:
> i've reported these on irc, but luka was not there
> so before i forget:

Thank you for your help with this!

> i've seen mixed use of %i and %d in printf
> i had to look up the difference (nothing)
> i recommend using only one of them consistently
> (i prefer %d)

I second this.  Luka - please use %d consistently.

There's a difference for scanf, but not for printf, and the convention
is to use %d with printf.

Alexander

Re: cluts comments

[Rich Felker]

On Wed, Aug 03, 2011 at 07:04:07PM +0200, Szabolcs Nagy wrote:
> alloc.c does not compile with glibc here
> it seems _POSIX_C_SOURCE is not enough for
> glibc to have SA_NODEFER in signal.h
> i guess that's a glibc bug..

We're halfway through 2011 and glibc can't catch up to POSIX 2008...
Previously SA_NODEFER was in the XSI option. Again, I think it would
be a very good idea to use -D_XOPEN_SOURCE=700 *globally* for all
tests.

> i've seen mixed use of %i and %d in printf
> i had to look up the difference (nothing)
> i recommend using only one of them consistently
> (i prefer %d)

Me too.

Rich

Re: cluts comments

[Luka Marčetić]

On 08/03/2011 07:04 PM, Szabolcs Nagy wrote:
> i've reported these on irc, but luka was not there
> so before i forget:
>
> alloc.c:127: WEXITSTAUS(stat) used after !WIFEXITED(stat)
>
> alloc.c should include signal.h (it uses sigaction)
>
> alloc.c does not compile with glibc here
> it seems _POSIX_C_SOURCE is not enough for
> glibc to have SA_NODEFER in signal.h
> i guess that's a glibc bug..

Thanks for the bug, Szabolcs. I'll correct it, and also add the header.
Not sure why you get an error for SA_NODEFER though. It compiles for me 
just fine with gcc version 4.6.1 (Debian 4.6.1-5) .
Note that alloc.c still crashes. I didn't know why yet ( possibly 
something very bad ;-) ). Same for setuid.c, which hangs. If you could 
find the reason for those two bugs, that would be awesome and I'd be 
very grateful. If not, I'll do it once I finish this darned generator.

> There's a difference for scanf, but not for printf, and the convention
> is to use %d with printf.
>
> Alexander

Well it is used more, I don't know about a convention... I do like %i 
better (though I was taught %d - hence the omissions). The reason why I 
like %i is that it implies the integer type. And that's what i have in 
alloc.c.
If you want me to change all %i to %d across all files, i can do that :-/
Luka

Re: cluts comments

[Rich Felker]

On Wed, Aug 03, 2011 at 07:38:30PM +0200, Luka Marčetić wrote:
> >alloc.c does not compile with glibc here
> >it seems _POSIX_C_SOURCE is not enough for
> >glibc to have SA_NODEFER in signal.h
> >i guess that's a glibc bug..
> 
> Thanks for the bug, Szabolcs. I'll correct it, and also add the header.
> Not sure why you get an error for SA_NODEFER though. It compiles for
> me just fine with gcc version 4.6.1 (Debian 4.6.1-5) .

I think he must be using an older version of glibc. It looks like they
fixed it to support POSIX 2008 better sometime this year.. Since it's
worthwhile to be able to test and evaluate regressions versus older
glibc versions, please make an effort to support them too if it's not
a big deal. (I thought the SA_NODEFER stuff was going to be removed
anyway, though..??)

> Well it is used more, I don't know about a convention... I do like
> %i better (though I was taught %d - hence the omissions). The reason
> why I like %i is that it implies the integer type. And that's what i
> have in alloc.c.

Yes, but %d makes explicit the base you want it printed in. :)
I'd never thought of %i as being useful in contrase with %f in code
that's mixing int/float and always using decimal, since I'm much more
often using integers only and more interested in the base...

Rich