[GRASS-dev] [GRASS GIS] #2762: more informative error messages in lib/rast/get_row.c

#2762: more informative error messages in lib/rast/get_row.c
---------------------------+-------------------------
Reporter: dylan | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: minor | Milestone: 7.1.0
Component: Raster | Version: svn-trunk
Keywords: error message | CPU: All
Platform: All |
---------------------------+-------------------------
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 [http://osgeo-
org.1560.x6.nabble.com/Error-reading-raster-data-for-row-xxx-only-when-
using-r-series-and-t-rast-series-td5229569.html on a couple] of
[http://lists.osgeo.org/pipermail/grass-dev/2015-July/075627.html
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);
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2762&gt;
GRASS GIS <https://grass.osgeo.org>

#2762: more informative error messages in lib/rast/get_row.c
--------------------------+---------------------------
  Reporter: dylan | Owner: grass-dev@…
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.1.0
Component: Raster | Version: svn-trunk
Resolution: | Keywords: error message
       CPU: All | Platform: All
--------------------------+---------------------------
Changes (by dylan):

* Attachment "get_row.c-patch" added.

patch with some suggested error messages, edits welcome

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2762&gt;
GRASS GIS <https://grass.osgeo.org>

#2762: more informative error messages in lib/rast/get_row.c
--------------------------+---------------------------
  Reporter: dylan | Owner: grass-dev@…
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.1.0
Component: Raster | Version: svn-trunk
Resolution: | Keywords: error message
       CPU: All | Platform: All
--------------------------+---------------------------

Comment (by neteler):

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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2762#comment:1&gt;
GRASS GIS <https://grass.osgeo.org>

#2762: more informative error messages in lib/rast/get_row.c
--------------------------+---------------------------
  Reporter: dylan | Owner: grass-dev@…
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.1.0
Component: Raster | Version: svn-trunk
Resolution: | Keywords: error message
       CPU: All | Platform: All
--------------------------+---------------------------

Comment (by 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.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2762#comment:2&gt;
GRASS GIS <https://grass.osgeo.org>

#2762: more informative error messages in lib/rast/get_row.c
--------------------------+---------------------------
  Reporter: dylan | Owner: grass-dev@…
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.1.0
Component: Raster | Version: svn-trunk
Resolution: | Keywords: error message
       CPU: All | Platform: All
--------------------------+---------------------------

Comment (by dylan):

Replying to [comment:2 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.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2762#comment:3&gt;
GRASS GIS <https://grass.osgeo.org>

#2762: more informative error messages in lib/rast/get_row.c
--------------------------+---------------------------
  Reporter: dylan | Owner: grass-dev@…
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.4.0
Component: Raster | Version: svn-trunk
Resolution: | Keywords: error message
       CPU: All | Platform: All
--------------------------+---------------------------

Comment (by 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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2762#comment:8&gt;
GRASS GIS <https://grass.osgeo.org>

#2762: more informative error messages in lib/rast/get_row.c
--------------------------+---------------------------
  Reporter: dylan | Owner: grass-dev@…
      Type: enhancement | Status: closed
  Priority: minor | Milestone: 7.4.0
Component: Raster | Version: svn-trunk
Resolution: fixed | Keywords: error message
       CPU: All | Platform: All
--------------------------+---------------------------
Changes (by dylan):

* status: new => closed
* resolution: => fixed

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2762#comment:9&gt;
GRASS GIS <https://grass.osgeo.org>

#2762: more informative error messages in lib/rast/get_row.c
--------------------------+---------------------------
  Reporter: dylan | Owner: grass-dev@…
      Type: enhancement | Status: reopened
  Priority: minor | Milestone: 7.4.0
Component: Raster | Version: svn-trunk
Resolution: | Keywords: error message
       CPU: All | Platform: All
--------------------------+---------------------------
Changes (by neteler):

* status: closed => reopened
* resolution: fixed =>

Comment:

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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2762#comment:10&gt;
GRASS GIS <https://grass.osgeo.org>

#2762: more informative error messages in lib/rast/get_row.c
--------------------------+---------------------------
  Reporter: dylan | Owner: grass-dev@…
      Type: enhancement | Status: reopened
  Priority: minor | Milestone: 7.4.0
Component: Raster | Version: svn-trunk
Resolution: | Keywords: error message
       CPU: All | Platform: All
--------------------------+---------------------------

Comment (by mmetz):

Replying to [comment:10 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.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2762#comment:11&gt;
GRASS GIS <https://grass.osgeo.org>

#2762: more informative error messages in lib/rast/get_row.c
--------------------------+---------------------------
  Reporter: dylan | Owner: grass-dev@…
      Type: enhancement | Status: reopened
  Priority: minor | Milestone: 7.4.1
Component: Raster | Version: svn-trunk
Resolution: | Keywords: error message
       CPU: All | Platform: All
--------------------------+---------------------------
Changes (by neteler):

* milestone: 7.4.0 => 7.4.1

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2762#comment:12&gt;
GRASS GIS <https://grass.osgeo.org>

#2762: more informative error messages in lib/rast/get_row.c
--------------------------+---------------------------
  Reporter: dylan | Owner: grass-dev@…
      Type: enhancement | Status: reopened
  Priority: minor | Milestone: 7.4.1
Component: Raster | Version: svn-trunk
Resolution: | Keywords: error message
       CPU: All | Platform: All
--------------------------+---------------------------

Comment (by martinl):

What exactly should be backported?

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2762#comment:13&gt;
GRASS GIS <https://grass.osgeo.org>

#2762: more informative error messages in lib/rast/get_row.c
--------------------------+---------------------------
  Reporter: dylan | Owner: grass-dev@…
      Type: enhancement | Status: reopened
  Priority: minor | Milestone: 7.4.1
Component: Raster | Version: svn-trunk
Resolution: | Keywords: error message
       CPU: All | Platform: All
--------------------------+---------------------------

Comment (by neteler):

Replying to [comment:13 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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2762#comment:14&gt;
GRASS GIS <https://grass.osgeo.org>