more informative error messages in lib/rast/get_row.c

[dylan]

In the file lib/rast/get_row.c there are several places in which the "Error reading raster data..." error message can be generated (grass_trunk r66508):

95:     G_fatal_error(_("Error reading raster data for row %d of <%s>"),
101:    G_fatal_error(_("Error reading raster data for row %d of <%s>"),
135:    G_fatal_error(_("Error reading raster data for row %d of <%s>"),
142:    G_fatal_error(_("Error reading raster data for row %d of <%s>"),
177:    G_fatal_error(_("Error reading raster data for row %d of <%s>"),
181:    G_fatal_error(_("Error reading raster data for row %d of <%s>"),
217:    G_fatal_error(_("Error reading raster data via GDAL for row %d of <%s>"),

It might be nice to know what exactly triggered these kind of messages, especially afer encountering these errors ​on a couple of ​occasions.

I would offer some text, but I don't really understand what is going on in code like this:

if (lseek(fcb->data_fd, t1, SEEK_SET) < 0)
	G_fatal_error(_("Error reading raster data for row %d of <%s>"),
		      row, fcb->name);

Something as simple as "Error reading raster data for row %d of <%s>\n file offset less than 0, ... blah blah blah" would IMHO help with debugging those modules that may have created such a file.

I would be happy to make the changes if someone is willing to comments on the meaning and implications of each case, outlined below:

if (lseek(fcb->data_fd, t1, SEEK_SET) < 0)
	G_fatal_error(_("Error reading raster data for row %d of <%s>"),
		      row, fcb->name);

 if ((size_t) G_zlib_read(fcb->data_fd, readamount, data_buf, bufsize) != bufsize)
	G_fatal_error(_("Error reading raster data for row %d of <%s>"),
		      row, fcb->name);

 if (read(fcb->data_fd, cmp, readamount) != readamount) {
	G_freea(cmp);
	G_fatal_error(_("Error reading raster data for row %d of <%s>"),
		      row, fcb->name);
    }

if (lseek(fcb->data_fd, (off_t) row * bufsize, SEEK_SET) == -1)
	G_fatal_error(_("Error reading raster data for row %d of <%s>"),
		      row, fcb->name);

if (read(fcb->data_fd, data_buf, bufsize) != bufsize)
	G_fatal_error(_("Error reading raster data for row %d of <%s>"),
		      row, fcb->name);

 if (err != CE_None)
	G_fatal_error(_("Error reading raster data via GDAL for row %d of <%s>"),
		      row, fcb->name);

[dylan]

patch with some suggested error messages, edits welcome

[neteler]

For the improved errors are a good idea. In the patch I would not use \n but simply a white space.

[wenzeslaus]

As noted in #2750, it might be easier to first apply patch from #2750 and then do these changes. Otherwise this patch makes absolute sense, different messages for different cases.

[dylan]

Replying to wenzeslaus:

As noted in #2750, it might be easier to first apply patch from #2750 and then do these changes. Otherwise this patch makes absolute sense, different messages for different cases.

I agree, please feel free it ignore my suggested patch for now. I'll re-write it using Markus' suggestions above after #2750 is addressed. I was thinking about adding some additional information such as buffer allocation vs. actual number of bytes read.

[neteler]

Milestone renamed

[neteler]

Ticket retargeted after milestone closed

[martinl]

All enhancement tickets should be assigned to 7.4 milestone.

[dylan]

We can probably close this ticket since Markus just added a more informative patch as of ​https://trac.osgeo.org/grass/ticket/2764#comment:8

[neteler]

Reopening since the ticket is tagged "7.4.0" but the messages are so far only in trunk. Is a backport desired?

[mmetz]

Replying to neteler:

Reopening since the ticket is tagged "7.4.0" but the messages are so far only in trunk. Is a backport desired?

This is not a bug-fix and we are at 7.4.0RC1, therefore I would recommend backporting to 7.4.1, together with the warnings added to lib/gis/compress.c and lib/gis/cmpr*.c.

[martinl]

What exactly should be backported?

[neteler]

Replying to martinl:

What exactly should be backported?

I think these msg improvements (from #3499):

• r72030 libraster: more informative messages when reading a row fails
• r72031 libgis: more informative messages when (de)compression fails
• r72042 libgis: add warnings when writing compressed data fails

[martinl]

All enhancement tickets should be assigned to 7.6 milestone.

[martinl]

Ticket retargeted after milestone closed

[martinl]

Ticket retargeted after milestone closed