Paul Kelly wrote:
Markus Metz wrote:
should
if (lseek(fd, 0L, SEEK_SET) == (off_t) - 1) {
not be
if (lseek(fd, 0L, SEEK_SET) == (off_t) -1) {
Hello Markus,
Well, looks like a bug in the indent program that got confused by the
cast and thought the - was being used as a binary rather than unary
operator. But is it not only an aesthetic problem? As far as I can see
the code should do exactly the same thing?
I am not sure if the code would do exactly the same thing with other
compilers than gcc, but that's beyond me.
if ((bytes_wrote = write(fd, &x, sizeof(int)) == sizeof(int)) < 0)
I am missing parentheses somewhere and would rather use
if ((bytes_wrote = write(fd, &x, sizeof(int))) != sizeof(int))
G_warning("%s", strerror(errno));
return (bytes_wrote == sizeof(int));
just to be on the safe side
Yes I suppose you saw too that the code that calls this function
assumes if the return value is non-zero that it succeeded, which
doesn't seem right. I would suggest to make it even clearer by
removing the redundant variable x, correcting the grammar (written
instead of wrote) and rearranging:
Index: format.c
===================================================================
--- format.c (revision 40905)
+++ format.c (working copy)
@@ -170,15 +170,12 @@
static int write_int(int fd, int n)
{
- int x;
- int bytes_wrote;
+ int bytes_written = write(fd, &n, sizeof(int));
- x = n;
-
- if ((bytes_wrote = write(fd, &x, sizeof(int)) == sizeof(int)) < 0)
+ if (bytes_written != sizeof(int))
G_warning("%s", strerror(errno));
- return bytes_wrote;
+ return (bytes_written == sizeof(int));
}
I think that is yet easier to read - do you agree?
Yes, that looks much better! AFAICT, ready to commit.
Markus M
_______________________________________________
grass-dev mailing list
[email protected]
http://lists.osgeo.org/mailman/listinfo/grass-dev
