[postgis-users] 1.3.3 Pre-Release Notice

[Mark Cave-Ayland]

On Tuesday 08 April 2008 22:37:34 Paul Ramsey wrote:
> OK, they should be gone now... btw what compiler are you using, I
> didn't get those...

Just standard GCC that comes with Debian testing:

$ gcc -v
Using built-in specs.
Target: x86_64-linux-gnu
Configured 
with: ../src/configure -v --enable-languages=c,c++,fortran,objc,obj-c++,treelang --prefix=/usr --enable-shared --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --enable-nls --with-gxx-include-dir=/usr/include/c++/4.2 --program-suffix=-4.2 --enable-clocale=gnu --enable-libstdcxx-debug --enable-objc-gc --enable-mpfr --disable-libmudflap --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 4.2.3 (Debian 4.2.3-2)

Taking a look at the committed fix, I see that you have the following:

if ( sr_id && strcmp(sr_id,"-1") ) printf("SRID=%s;", sr_id);

I have a very strong feeling that evaluation order in C is unspecified, rather 
than being from left to right; so for example the compiler could decide to 
generate code that evaluates the strcmp() first, in which case it would 
segfault on a NULL :( I think you would need to re-write something like this:

if (sr_id)
	if (strcmp(sr_id,"-1") ) printf("SRID=%s;", sr_id);

(looks even closer...)

In fact, why is sr_id stored as a string anyway? We should make sr_id an 
integer with a default value of -1, and then use sscanf() within the switch() 
statement of pgis_getopt() to read its value.


ATB,

Mark.

-- 
Mark Cave-Ayland
Sirius Corporation - The Open Source Experts
http://www.siriusit.co.uk
T: +44 870 608 0063