[Gdal-dev] Assertion in .tab driver

[Daniel Morissette]

Thanks for providing those details Daniel, that helped a lot. FYI a 
ticket has been filed and this is now fixed in in the MITAB driver for 
both the trunk and 1.4 branch in SVN:

https://trac.osgeo.org/gdal/ticket/1678

I have also created ticket 1681 about fixing all isspace() calls in GDAL:
https://trac.osgeo.org/gdal/ticket/1681

Daniel

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
> 
> On 6/14/07, Daniel Morissette <dmorissette at mapgears.com> wrote:
>> I've received a contributed patch recently to replace this same
>> isspace() call with a call to iswspace(), claiming that isspace() could
>> potentially "crash" when passed a wide char. The change is not in GDAL
>> yet, it will be ported to the GDAL copy of the MITAB driver before the
>> GDAL 1.5.0 release.
>>
>> I wonder if his "crash" might have been the same assert that you
>> encountered and if yes then which fix between yours and his is better. I
>> have contacted him to get his thoughts but haven't received any reply
>> yet. I'd welcome your comments too.
>>
>> Daniel
>>
>>
>>
>> Daniel wrote:
>> > 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
>> >
>> > _______________________________________________
>> > Gdal-dev mailing list
>> > Gdal-dev at lists.maptools.org
>> > http://lists.maptools.org/mailman/listinfo/gdal-dev
>>
>>
>> -- 
>> Daniel Morissette
>> http://www.mapgears.com/
>> _______________________________________________
>> Gdal-dev mailing list
>> Gdal-dev at lists.maptools.org
>> http://lists.maptools.org/mailman/listinfo/gdal-dev
>>
> 
> _______________________________________________
> Gdal-dev mailing list
> Gdal-dev at lists.maptools.org
> http://lists.maptools.org/mailman/listinfo/gdal-dev


-- 
Daniel Morissette
http://www.mapgears.com/