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) {
The two are equivalent; the whitespace isn't significant here. > because I can't see where 1 is subtracted from? Introduced with > automated indentation in r32527 There is no subtraction; the "-" is treated as a unary minus (negation) operator in this context. If you were to change it to something which cannot be interpreted as a unary operator (e.g. / or %), you would get an error. > 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) Ouch. There's no way that that can make sense. As it stands, it's parsed as: if ((bytes_wrote = (write(fd, &x, sizeof(int)) == sizeof(int))) < 0) i.e. bytes_wrote will be either 0 or 1, so the "... < 0" will always be false. > 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)); I would use: static int write_int(int fd, int n) { if (write(fd, &n, sizeof(int)) != sizeof(int)) { G_warning("%s", strerror(errno)); return 0; } return 1; } There's no need to make a copy of "n"; it's already a copy of the actual argument. Actually, looking at the code in context, I'd write: static void write_int(int fd, int n) { if (write(fd, &n, sizeof(int)) != sizeof(int)) G_fatal_error("%s", strerror(errno)); } Lets face it: you aren't going to be recovering from this situation, are you? If _segment_format fails, the module fails. While you're there, can you fix this nonsense: if (file_size > INT_MAX) { ? OTOH, the segment library is long overdue for being replaced with something more efficient. -- Glynn Clements <[email protected]> _______________________________________________ grass-dev mailing list [email protected] http://lists.osgeo.org/mailman/listinfo/grass-dev
