Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#1678 closed defect (fixed)

isspace() causes failed assertion in .tab driver

Reported by: Daniel Morissette Owned by: Daniel Morissette
Priority: normal Milestone: 1.4.2
Component: OGR_SF Version: unspecified
Severity: normal Keywords:
Cc: warmerdam

Description

On June 14th, 2007, Daniel Bäck wrote to gdal-dev:

> Hello,
>
> If you build GDAL against the debug version of the c-runtime with
> Visual Studio 2005 (/MDd) and try to open a .tab file with 8-bit ASCII
> characters in it, the field names for instance, you get an assert in
> isspace on row 177 in
> http://trac.osgeo.org/gdal/browser/trunk/gdal/ogr/ogrsf_frmts/mitab/mitab_imapinfofile.cpp
>
> The parameter passed to isspace is a signed char which is converted to
> a signed int so any values >127 will become negative. It seems to work
> fine when built against the release libraries and if you ignore the
> assert it seems to work so this is probably nothing serious maybe
> depending on the locale that is used. It can easily be fixed by
> casting the char to an unsigend char before passing it to isspace:
> isspace((unsigned char)*pszLine)
>
> Regards,
>  Daniel Bäck 

Change History (3)

comment:1 Changed 12 years ago by Daniel Morissette

Status: newassigned

MITAB bug 1737 already proposed changing this same isspace() to iswspace(): http://bugzilla.maptools.org/show_bug.cgi?id=1737

After bringing up this issue, Daniel sent the following (and very useful) reply:

Daniel wrote:
> If he got a crash it could very well  be the same problem since
> isspace expects input that can be represented as an unsigned char or
> EOF.
> 
> Looking at Microsofts documentation for isspace at:
> http://msdn2.microsoft.com/en-us/library/y13z34da(VS.80).aspx
> 
> "When used with a debug CRT library, isspace will display a CRT assert
> if passed a parameter that is not EOF or in the range of 0 through
> 0xFF. When used with a non-debug CRT library, isspace will use the
> parameter as an index into an array, with undefined results if the
> parameter is not EOF or in the range of 0 through 0xFF."
> 
> So this could very well cause a crash. I don't think that the correct
> solution is to use iswspace since the rest of the code can't handle
> wide chars anyway.
> 
> Some locales might even have characters >127 that should be treated as
> whitespace and changing to use iswspace won't fix that.
> 
> This seems to be quite a common problem, see "C Language Gotchas"
> http://www.greenend.org.uk/rjk/2001/02/cfu.html#ctypechar
> 
> I did a quick search for isspace in the GDAL source and there seems to
> be some more places where isspace is passed a char instead of an
> unsigned char.
> 
> Regards,
>  Daniel Bäck

comment:2 Changed 12 years ago by Daniel Morissette

Resolution: fixed
Status: assignedclosed

I have fixed this in both trunk (r11686) and branches/1.4 (r11687) by adding the (unsigned char) cast to all isspace() calls inside MITAB.

comment:3 Changed 12 years ago by Daniel Morissette

Milestone: 1.4.2
Note: See TracTickets for help on using tickets.