[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Findutils-patches] [PATCH] Include sys/stat.h in files where we use
Re: [Findutils-patches] [PATCH] Include sys/stat.h in files where we use struct stat.
Tue, 26 Jun 2007 06:51:14 -0600
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:220.127.116.11) Gecko/20070509 Thunderbird/18.104.22.168 Mnenhy/0.7.5.666
-----BEGIN PGP SIGNED MESSAGE-----
According to James Youngman on 6/26/2007 2:01 AM:
> On 6/25/07, Eric Blake <address@hidden> wrote:
>> That looks a bit odd to me, with extra spacing and parenthesis. I think
>> GNU coding standards prefer:
>> assert (AT_FDCWD == curr_fd || curr_fd >= 0);
> I have no objection to that style particularly, but I prefer people
> who see that the assertion failed not to have to check their mental
> operator precedence list.
Reasonable enough to keep the inner parenthesis when they are
user-visible. However, there's still the question of whether
assert ( (cond1) || (cond2) );
assert ((cond1) || (cond2));
looks nicer, but a quick check shows that, for gcc 3.4.4 at least, the
leading and trailing space of the former are not stringized.
> I agree with you on the issue of NULL, but I am in firm diagreement on
> the issue of '\0'. As I am sure you know, in C the type of '\0' is
> int, not char. Hence '\0' and 0 are of the same type and have the
> same value. Choice between them is essentially stylistic.
> But I have found in the past that typos are a frequent source of bugs.
> The best-known case of this is the accidental change of '==' to '='.
> However, in this case a typo can change '\0' to '0', introducing a
> bug. For this reason I prefer to use 0 rather than '\0' because the
> former isn't vulnerable to bugs introduced as typos.
You provide a nice argument. Maybe it's worth documenting that in the
findutils maintainers note, since the GNU Coding Standards are silent on
preference between 0 and '\0'.
>> A comment here that you are intentionally casting away void might be
> If you mean casting away const, that was a bug, thanks for finding it,
> but it was fixed in a later patch.
Yes, that's what I meant.
> I have an idea that it might be standard-conforming to assign the
> function pointer values to a union member and use memcmp(). What do
> you think?
As in the following? Yes, I think this is safer:
char mem[sizeof (pred_cost_lookup*)];
} u1, u2;
u1.fn = p1;
u2.fn = p2;
return memcmp (u1.mem, u2.mem, sizeof (pred_cost_lookup*));
Don't work too hard, make some time for fun as well!
Eric Blake address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
-----END PGP SIGNATURE-----