CPLScanPointer doesn't do its job rightly when compiled in MSYS environment

[ajolma]

CPLScanPointer uses

#if defined(WIN32) && defined(_MSC_VER)

to test whether to give sscanf a hex without the 0x prefix. The requirement is (probably) a feature of sscanf in msvcrt.dll. This does not work for MSYS, which does not define _MSC_VER. Instead MSYS (MinGW gcc?) defines MSVCRT to indicate that it will be using msvcrt.dll (and not msys-1.0.dll).

The question is now how to fix this. In this case the issue is the runtime that is used and not the compiler. If _MSC_VER checks for the compiler, then it shouldn't be used here. See similar discussion here:  http://mail.gnome.org/archives/xml/2002-October/msg00002.html Maybe we should use MSVCRT every time the runtime is the issue and somewhere in configuration add

#ifdef _MSC_VER
#define __MSVCRT__
#endif

[rouault]

Ari,

your idea about distinguishing between compiler and runtime library make sense. But for that particular issue about CPLScanPointer, what would you say about :

    if( EQUALN(szTemp,"0x",2) )
    {
        pResult = NULL;
        /* With some runtime libraries, sscanf doesn't like the 0x prefix */
        /* So do a quick check to determine if we must strip the prefix or not */
        sscanf( "0x12345678", "%p", &pResult );
        if (pResult == (void*) 0x12345678)
            sscanf( szTemp, "%p", &pResult );
        else
            sscanf( szTemp+2, "%p", &pResult );
    }

This is similar in spirit to what is already currently done in CPLPrintPointer. The advantage is that we don't even need to worry about what other strange environment would do in such situations...

[ajolma]

I'm not against testing sscanf in runtime.

Meanwhile I've found out that there's an issue with printf in MSYS. While it has long long because it's gcc, it requires %I64d because it uses msvcrt. This causes problems for example in tif_print.c. There's a nice abstraction cpl_port.h, which could be exploited and we wouldn't need those defines in tif_print.c and elsewhere, there's on for example cpl_conv.cpp. (Maybe libtiff in GDAL is downstream and the logics in cpl_port.h should be copied).

BTW, I don't understand why

#if defined(WIN32) && defined(_MSC_VER)

is used. Don't _MSC_VER *always* imply WIN32? Or am I missing something?

[ajolma]

My suggestions are thus, for cpl_port.h section 64bit support:

#if defined(_MSC_VER)

#define VSI_LARGE_API_SUPPORTED
typedef __int64          GIntBig;
typedef unsigned __int64 GUIntBig;

#elif HAVE_LONG_LONG

typedef long long        GIntBig;
typedef unsigned long long GUIntBig;

#else

typedef long             GIntBig;
typedef unsigned long    GUIntBig;

#endif

#if defined(__MSVCRT__)

#define CPL_FRMT_GB     "I64"

#elif HAVE_LONG_LONG

#define CPL_FRMT_GB     "ll"

#else

#define CPL_FRMT_GB     "l"

#endif

The sprintf statement in CPLPrintUIntBig:

sprintf( szTemp, "%*"CPL_FRMT_GB"u", nMaxLen, iValue );

[rouault]

I'm not sure that #if defined(WIN32) && defined(_MSC_VER) is redundant, because of WinCE... I think that for WinCE, WIN32 is not defined, but _MSC_VER is.

When I read, the first few lines of cpl_port.h, my understanding is that :

• WIN32 is defined (by CPL) because of _WIN32 being defined and not _WIN32_WCE
• WIN32CE is defined (by CPL) because of _WIN32_WCE being defined.

So it would mean that WIN32 is not defined for WinCE . However, in cpl_vsil_win32.cpp, the code inside the #if defined(WIN32) could let us think that we could go here even in the WinCE case...

I'm lost. All of this has been introduced in r9210. Mateusz, could you bring up some light on the subject ? To sum up :

• is WIN32 defined in the WinCE case ? Or is it just WIN32CE ?
• Can we expect int64, _atoi64, "%I64d" being defined on WinCE ?

[ajolma]

One more MSVCRT issue: _MSC_VER at iom_utilities.cpp:55 should be MSVCRT

I'd like to move forward with this issue before the todo list becomes too long and my copy of GDAL for MSYS differs too much from trunk. I'm not sure the latest issue I brought up which added the WinCE complication is needed and we could move on with more clear things like this one.

[rouault]

Ari, in r16702 I've commited variations on the above discussions to improve mingw compatibility. Could you test ?

[ajolma]

Thanks, looks good but I'll test a bit later.

Shouldn't you use "u" instead of "d" in CPLPrintUIntBig? iValue is GUIntBig.

[warmerdam]

I have had to alter the iom_utilities.cpp code to not assume that MSVCRT will be defined by MSVC (r16745).

[ajolma]

I'm generally happy with what Even commited in r16702. Only two things remain:

pcidsk dataset code within DEBUG (pcidskdataset.cpp:704 and below) (I cc this to dron, who is the author of pcidskdataset), and

libtiff, which uses in several places %I64d conditioned only with WIN32 and MSC. IMO libtiff code would also become much nicer in all those places if it would/could use GUIntBig, CPL_FRMT_GIB, and CPL_FRMT_GUIB. I'll post a message on the libtiff list.

But otherwise, as a GDAL bug I believe this is fixed.