minaco2 / distcc

Automatically exported from code.google.com/p/distcc
GNU General Public License v2.0
0 stars 0 forks source link

should use S_ISLNK and S_ISDIR #9

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
it's not necessarily correct to test (mode & S_IFLNK) == S_IFLNK or (mode &
S_IFDIR) == S_IFDIR; you want to test all the S_IFMT bits. easiest to use
the provided S_ISLNK and S_ISDIR macros...

Original issue reported on code.google.com by elliott....@gmail.com on 25 Jun 2008 at 1:08

Attachments:

GoogleCodeExporter commented 9 years ago
Why is it not correct? If you expand out the S_ISDIR macro, it basically does 
the
same thing.

Original comment by zhang...@gmail.com on 1 Jul 2008 at 1:01

GoogleCodeExporter commented 9 years ago
no; expand out the S_ISDIR macro, and you'll see the difference between the 
broken
code in distcc and the S_ISDIR macro is exactly what i said above: you need to 
use (m
& S_IFMT).

taking the example of S_IFDIR...

good code: ((m & S_IFMT) == S_IFDIR)
distcc:    ((m & S_IFDIR) == S_IFDIR)

if you're saying you don't understand why they're not equivalent, try 
substituting in
S_IFSOCK for m.

but, like i said, best of all is to use S_ISLNK and S_ISDIR because someone else
worries about whether they're correct, and they're more intention-revealing 
anyway.

Original comment by elliott....@gmail.com on 1 Jul 2008 at 3:50

GoogleCodeExporter commented 9 years ago
Ok, the results will be different, but only if stat() reports the file is both a
directory and a {fifo, char device, etc...}. The chances of that happening are 
very
rare, and if that ever happens, you probably have more serious problems to 
worry about.

I agree using the macros makes the code more readable. Please send the patch to:
distcc-patches at googlegroups.com. I don't know if patches get picked up from 
here.
See: http://lists.samba.org/archive/distcc/2008q2/003670.html

Original comment by zhang...@gmail.com on 1 Jul 2008 at 7:12

GoogleCodeExporter commented 9 years ago
I've applied the patch.

Original comment by fergus.h...@gmail.com on 30 Jul 2008 at 1:23