Opened 14 years ago

Closed 14 years ago

#3405 closed defect (fixed)

Geoconcept reader fails with some geometries

Reported by: gaige Owned by: warmerdam
Priority: normal Milestone: 1.8.0
Component: OGR_SF Version: svn-trunk
Severity: normal Keywords: geoconcept
Cc: dgrichard

Description

This may only affect computers with certain compilers, however it definitely did affect mine.

Reading lines with iYP columns would fail because the test for iGr-iYP==1 would fail due to testing iGr-iY instead (which will fail in these circumstances.

Chances are that the parenthesis are all that is necessary here, but the flipping of the conditionals makes it more clear what is going on.

ogr/ogrsf_frmts/geoconcept/geoconcept.c:
1306c1306
<                   if( (iYP!=-1 && iGr-iYP!=1) || (iGr-iY!=1) )
---
> 				  if(!(((iYP==-1) && (iGr-iY==1)) ||(iGr-iYP==1)))

Change History (7)

comment:1 by Even Rouault, 14 years ago

Cc: dgrichard added
Keywords: geoconcept added

CC'ing Didier Richard, the author of the Geoconcept driver

comment:2 by dgrichard, 14 years ago

I will give a glance asap

in reply to:  description ; comment:3 by dgrichard, 14 years ago

Replying to gaige:

ogr/ogrsf_frmts/geoconcept/geoconcept.c:
1306c1306
<                   if( (iYP!=-1 && iGr-iYP!=1) || (iGr-iY!=1) )
---
> 				  if(!(((iYP==-1) && (iGr-iY==1)) ||(iGr-iYP==1)))

I would propose this :

ogr/ogrsf_frmts/geoconcept/geoconcept.c:
1306c1306
<                   if( (iYP!=-1 && iGr-iYP!=1) || (iGr-iY!=1) )
---
> 				  if( !( ((iGr!=-1) && ( (iGr==iY+1) || (iGr==iYP+1) )) || (iGr==-1) ) )

which better reflects that if the Graphics field is set (iGr!=-1) then its rank must follow either the abscissa rank or the last abscissa rank in the fields declaration list.

Does it work better with your compiler ?

in reply to:  3 ; comment:4 by gaige, 14 years ago

Replying to dgrichard:

Replying to gaige:

which better reflects that if the Graphics field is set (iGr!=-1) then its rank must follow either the abscissa rank or the last abscissa rank in the fields declaration list.

Does it work better with your compiler ?

Yep, that seems to function just fine and makes more sense.

Do you want to put it in, or shall I?

in reply to:  4 comment:5 by dgrichard, 14 years ago

Replying to gaige:

Yep, that seems to function just fine and makes more sense.

Do you want to put it in, or shall I?

I will put it in

Thanks for the report.

comment:6 by dgrichard, 14 years ago

fixed in r18980

comment:7 by Even Rouault, 14 years ago

Milestone: 1.7.21.8.0
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.