On Sat, 26 Aug 2006, Jonas Fonseca wrote:
>
> diff --git a/dir.c b/dir.c
> index d53d48f..ff8a2fb 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -122,11 +122,11 @@ static int add_excludes_from_file_1(cons
> size = lseek(fd, 0, SEEK_END);
> if (size < 0)
> goto err;
> - lseek(fd, 0, SEEK_SET);
> if (size == 0) {
> close(fd);
> return 0;
> }
> + lseek(fd, 0, SEEK_SET);
I really think you'd be better off rewriting that to use "fstat()"
instead. I don't know why it uses two lseek's, but it's wrong, and looks
like some bad habit Junio picked up at some point.
> @@ -146,7 +146,7 @@ static int add_excludes_from_file_1(cons
> return 0;
>
> err:
> - if (0 <= fd)
> + if (0 >= fd)
> close(fd);
That's wrong.
Now, admittedly it's wrong because another bad habit Junio picked up
(doing comparisons with constants in the wrong order), so please write it
as
if (fd >= 0)
close(fd);
instead.
Junio: I realize that you claim that you learnt that syntax from an
authorative source, but he was _wrong_. Doing the constant first is more
likely to cause bugs, rather than less. Compilers will warn about the
if (x = 0)
..
kind of bug, and putting the constant first just confuses humans.
It's more important to _not_ confuse humans than it is to try to avoid an
uncommon error that compilers can and do warn about anyway.
Linus
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html