[GRASS-dev] New Defects reported by Coverity Scan for grass

GRASS GIS GRASS GIS developer
1

Hi,

I have submitted r62566 from today to Coverity Scan, results below.

Markus

---------- Forwarded message ----------
From: <scan-admin@coverity.com>
Date: Mon, Nov 3, 2014 at 9:57 AM
Subject: New Defects reported by Coverity Scan for grass
To: neteler@osgeo.org

Hi,

Please find the latest report on new defect(s) introduced to grass
found with Coverity Scan.

5 new defect(s) introduced to grass found with Coverity Scan.
26 defect(s), reported by Coverity Scan earlier, were marked fixed in
the recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 5 of 5 defect(s)

** CID 1207286: Unchecked return value (CHECKED_RETURN)
/imagery/i.atcorr/main.cpp: 419 in copy_colors(const char *, char *)()

** CID 1207285: Unchecked return value (CHECKED_RETURN)
/lib/ogsf/gs3.c: 592 in Gs_build_256lookup()

** CID 1207284: Unchecked return value (CHECKED_RETURN)
/lib/ogsf/gs3.c: 654 in Gs_pack_colors()

** CID 1207283: Unchecked return value (CHECKED_RETURN)
/lib/ogsf/gs3.c: 725 in Gs_pack_colors_float()

** CID 1207209: Unchecked return value (CHECKED_RETURN)
/imagery/i.pca/support.c: 19 in write_support()

________________________________________________________________________________________________________
*** CID 1207286: Unchecked return value (CHECKED_RETURN)
/imagery/i.atcorr/main.cpp: 419 in copy_colors(const char *, char *)()
413
414 /* Copy the colors from map named iname to the map named oname */
415 static void copy_colors(const char *iname, char *oname)
416 {
417 struct Colors colors;
418

    CID 1207286: Unchecked return value (CHECKED_RETURN)
    Calling "Rast_read_colors" without checking return value (as is done elsewhere 50 out of 61 times).

419 Rast_read_colors(iname, "", &colors);
420 Rast_write_colors(oname, G_mapset(), &colors);
421 }
422
423
424 /* Define our module so that Grass can print it if the user
wants to know more. */

________________________________________________________________________________________________________
*** CID 1207285: Unchecked return value (CHECKED_RETURN)
/lib/ogsf/gs3.c: 592 in Gs_build_256lookup()
586 mapset = G_find_raster2(filename, "");
587 if (!mapset) {
588 G_warning(_("Raster map <%s> not found"), filename);
589 return 0;
590 }
591

    CID 1207285: Unchecked return value (CHECKED_RETURN)
    Calling "Rast_read_colors" without checking return value (as is done elsewhere 50 out of 61 times).

592 Rast_read_colors(filename, mapset, &colrules);
593 Rast_get_c_color_range(&min, &max, &colrules);
594
595 if (min < 0 || max > 255) {
596 G_warning(_("Color table range doesn't match data
(mincol=%d, maxcol=%d"),
597 min, max);

________________________________________________________________________________________________________
*** CID 1207284: Unchecked return value (CHECKED_RETURN)
/lib/ogsf/gs3.c: 654 in Gs_pack_colors()
648
649 r = (unsigned char *)G_malloc(cols);
650 g = (unsigned char *)G_malloc(cols);
651 b = (unsigned char *)G_malloc(cols);
652 set = (unsigned char *)G_malloc(cols);
653

    CID 1207284: Unchecked return value (CHECKED_RETURN)
    Calling "Rast_read_colors" without checking return value (as is done elsewhere 50 out of 61 times).

654 Rast_read_colors(filename, mapset, &colrules);
655
656 cur = buff;
657
658 G_message(_("Translating colors from raster map <%s>..."),
659 G_fully_qualified_name(filename, mapset));

________________________________________________________________________________________________________
*** CID 1207283: Unchecked return value (CHECKED_RETURN)
/lib/ogsf/gs3.c: 725 in Gs_pack_colors_float()
719
720 r = (unsigned char *)G_malloc(cols);
721 g = (unsigned char *)G_malloc(cols);
722 b = (unsigned char *)G_malloc(cols);
723 set = (unsigned char *)G_malloc(cols);
724

    CID 1207283: Unchecked return value (CHECKED_RETURN)
    Calling "Rast_read_colors" without checking return value (as is done elsewhere 50 out of 61 times).

725 Rast_read_colors(filename, mapset, &colrules);
726
727 fcur = fbuf;
728 icur = ibuf;
729
730 G_message(_("Translating colors from raster map <%s>..."),

________________________________________________________________________________________________________
*** CID 1207209: Unchecked return value (CHECKED_RETURN)
/imagery/i.pca/support.c: 19 in write_support()
13 const char *mapset = G_mapset();
14 struct Colors colors;
15 struct FPRange range;
16 DCELL min, max;
17
18 if (inname) {

    CID 1207209: Unchecked return value (CHECKED_RETURN)
    Calling "Rast_read_colors" without checking return value (as is done elsewhere 50 out of 61 times).

19 Rast_read_colors(inname, "", &colors);
20 }
21 else {
22 /* make grey scale color table */
23 Rast_read_fp_range(outname, mapset, &range);
24 Rast_get_fp_range_min_max(&range, &min, &max);

________________________________________________________________________________________________________
To view the defects in Coverity Scan visit,
http://scan.coverity.com/projects/1038?tab=overview

2

Markus Neteler wrote:

I have submitted r62566 from today to Coverity Scan, results below.

>>> Calling "Rast_read_colors" without checking return value (as is
done elsewhere 50 out of 61 times).

I'd suggest fixing this by removing the redundant checks and making
the needed check mostly redundant.

While most modules check the return code, exactly what they check for
(==-1, <0, <=0) isn't consistent.

Rast_read_colors() either:

1. Successfully reads a colour table and returns 1.

2. Discovers that the map lacks a colour table, generates a default
rainbow colour table, issues a warning, and returns -2.

3. Discovers that the map's colour table is syntactically invalid,
issues a warning, and returns -1.

The first case isn't an error.

The second case isn't really an error for most modules.

The third case indicates an actual problem (the colour table won't
have been initialised) but it would be straightforward to substitute a
default colour table as per the second case.

For the second and third cases, the module can check the return code
if a default colour table means that its output is going to be garbage
(e.g. modules which use it to translate cell values to a 0..1
intensity value). Otherwise, it can just proceed using the default
colour table.

--
Glynn Clements <glynn@gclements.plus.com>