[GRASS-dev] [GRASS GIS] #2438: Vect_get_line_type cannot handle line out of range

#2438: Vect_get_line_type cannot handle line out of range
-------------------------+--------------------------------------------------
Reporter: artegion | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 6.4.5
Component: LibVector | Version: unspecified
Keywords: | Platform: Unspecified
      Cpu: Unspecified |
-------------------------+--------------------------------------------------
calling Vect_get_line_type with line out of range (i.e. line=0)
Vect_line_alive returns -1 but Vect_get_line_type ignores it (not -1 is
False just like not 1) -> access violation reading ....

{{{
>>> from grass.lib.vector import Vect_line_alive
>>> from grass.lib.vector import Vect_get_line_type
>>> from grass.pygrass vector import VectorTopo
>>> a= VectorTopo('test')
>>> a.open()
>>> Vect_line_alive(a.c_mapinfo, 1)
1
>>> Vect_get_line_type(a.c_mapinfo, 1)
1
>>> Vect_line_alive(a.c_mapinfo, 0)
-1
>>> not Vect_line_alive(a.c_mapinfo, 0)
False
>>> Vect_get_line_type(a.c_mapinfo, 0)
Traceback (most recent call last):
   File "<stdin>", line 1, in <module>
WindowsError: exception: access violation reading 0x736C0014
>>>
}}}

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

#2438: Vect_get_line_type cannot handle line out of range
--------------------------+-------------------------------------------------
  Reporter: artegion | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: normal | Milestone: 6.4.5
Component: LibVector | Version: unspecified
Resolution: invalid | Keywords:
  Platform: Unspecified | Cpu: Unspecified
--------------------------+-------------------------------------------------
Changes (by mmetz):

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

Comment:

Replying to [ticket:2438 artegion]:
> calling Vect_get_line_type with line out of range (i.e. line=0)
> Vect_line_alive returns -1 but Vect_get_line_type ignores it (not -1 is
False just like not 1) -> access violation reading ....

This is a programmer's error. 'Vect_get_line_type()' assumes that the
programmer first tested if the provided line id is indeed valid. You did
call 'Vect_line_alive()' first, you can only proceed if the line is alive.
Fix your code.

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

#2438: Vect_get_line_type cannot handle line out of range
--------------------------+-------------------------------------------------
  Reporter: artegion | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: normal | Milestone: 6.4.5
Component: LibVector | Version: unspecified
Resolution: | Keywords:
  Platform: Unspecified | Cpu: Unspecified
--------------------------+-------------------------------------------------
Changes (by artegion):

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

Comment:

Replying to [comment:1 mmetz]:
> Replying to [ticket:2438 artegion]:
> > calling Vect_get_line_type with line out of range (i.e. line=0)
> > Vect_line_alive returns -1 but Vect_get_line_type ignores it (not -1
is False just like not 1) -> access violation reading ....
>
> This is a programmer's error. 'Vect_get_line_type()' assumes that the
programmer first tested if the provided line id is indeed valid. You did
call 'Vect_line_alive()' first, you can only proceed if the line is alive.
Fix your code.

For sure calling 'Vect_get_line_type()' with a wrong line id is a
programmer error but 'Vect_get_line_type()' already call
'Vect_line_alive()'.

{{{
   258 int Vect_get_line_type(const struct Map_info *Map, int line)
   259 {
   260 check_level(Map);
   261
   262 if (!Vect_line_alive(Map, line))
   263 return 0;
   264
   265 return (Map->plus.Line[line]->type);
   266 }
}}}

Testing if (!Vect_line_alive(Map, line)) is wrong because Vect_line_alive
returns[[BR]]

1 feature alive[[BR]]

0 feature is dead[[BR]]

-1 on error[[BR]]

a small change ( if (Vect_line_alive(Map, line!=1)) may improve code
robustness.

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

#2438: Vect_get_line_type cannot handle line out of range
--------------------------+-------------------------------------------------
  Reporter: artegion | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: normal | Milestone: 6.4.5
Component: LibVector | Version: unspecified
Resolution: | Keywords:
  Platform: Unspecified | Cpu: Unspecified
--------------------------+-------------------------------------------------

Comment(by artegion):

sorry: if (Vect_line_alive(Map, line)!=1)

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

#2438: Vect_get_line_type cannot handle line out of range
--------------------------+-------------------------------------------------
  Reporter: artegion | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: normal | Milestone: 7.0.0
Component: LibVector | Version: unspecified
Resolution: | Keywords:
  Platform: Unspecified | Cpu: Unspecified
--------------------------+-------------------------------------------------
Changes (by mmetz):

  * milestone: 6.4.5 => 7.0.0

Comment:

Replying to [comment:2 artegion]:
> Replying to [comment:1 mmetz]:
> > Replying to [ticket:2438 artegion]:
> > > calling Vect_get_line_type with line out of range (i.e. line=0)
> > > Vect_line_alive returns -1 but Vect_get_line_type ignores it (not -1
is False just like not 1) -> access violation reading ....
> >
> > This is a programmer's error. 'Vect_get_line_type()' assumes that the
programmer first tested if the provided line id is indeed valid. You did
call 'Vect_line_alive()' first, you can only proceed if the line is alive.
Fix your code.
>
> For sure calling 'Vect_get_line_type()' with a wrong line id is a
programmer error but 'Vect_get_line_type()' already call
'Vect_line_alive()'.
>
>
> Testing if (!Vect_line_alive(Map, line)) is wrong because
Vect_line_alive returns[[BR]]
>
> 1 feature alive[[BR]]
>
> 0 feature is dead[[BR]]
>
> -1 on error[[BR]]
>
>
> a small change ( if (Vect_line_alive(Map, line!=1)) may improve code
robustness.

The problem comes from r55582 which introduced return -1 on error for the
Vect_*_alive functions. Unfortunately, the existing calls to
Vect_*_alive() were not adjusted. I have restored the behaviour of the
Vect_*_alive() functions in r62370,1, no more need to change
Vect_get_line_type(). Note that this applies only to G7, not to G6.

>
>
>

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2438#comment:4&gt;
GRASS GIS <http://grass.osgeo.org>