On Wed, 10 Feb 2010, Markus Metz wrote:
I found two code snippets in the segment library (all branches) that look
fishy to me:
the first is in relbr6 here
https://trac.osgeo.org/grass/browser/grass/branches/releasebranch_6_4/lib/segment/format.c#L135
should
if (lseek(fd, 0L, SEEK_SET) == (off_t) - 1) {
not be
if (lseek(fd, 0L, SEEK_SET) == (off_t) -1) {
because I can't see where 1 is subtracted from? Introduced with automated
indentation in r32527
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?
the second is in relbr6 here:
https://trac.osgeo.org/grass/browser/grass/branches/releasebranch_6_4/lib/segment/format.c#L178
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?
Paul
_______________________________________________
grass-dev mailing list
[email protected]
http://lists.osgeo.org/mailman/listinfo/grass-dev
