Ticket #3528 (closed defect: fixed)

Opened 5 years ago

Last modified 5 years ago

Building on x64 Windows

Reported by: ripleybd Owned by: warmerdam
Priority: normal Milestone:
Component: ConfigBuild Version: unspecified
Severity: normal Keywords:
Cc:

Description

I needed to make some changes to build for x64 Windows. I used the cross-compilers from x86_64 Linux from the MinGW-w64 project (as used for R: these reports all come from helping Roger Bivand with the R binding for GDAL).

'ar' is hardcoded in frmts/pcidsk/sdk/Makefile: it should be $(AR).

There is a fairly pervasive design error that a pointer can be fitted into an unsigned long: this is not so on x64 Windows where long is 32 bit. The modern solution is to use uintptr_t, and that is what I have used in the attached patch. That is C99, but as you are using it for C++ I suggest that for portability it needs to be tested for in configure and otherwise typedef-ed suitably (unsigned long or unsigned long long, although the latter is not portable and rejected by g++ -pedantic).

Attachments

W64gdal.patch Download (7.4 KB) - added by ripleybd 5 years ago.

Change History

Changed 5 years ago by ripleybd

Changed 5 years ago by rouault

As far as I know, frmts/pcidsk/sdk/Makefile is not used when building GDAL. It is only used if you want building pcidsk lib as a standalone lib.

For the pointer issue, this has been reported in ticket #3460 and fixed it a bit differently. I agree with you that the best type should be uintptr_t, and that's what should be used for the future. But for API/ABI stability reasons (cpl_hash_set), I think it's better to keep long and we've modified the casts that stopped the code from compiling. For a hash value, there's no harm if we just keep the low 32 bits (small risk of more hash collisions). For eErr = (CPLErr) (long) papThreadDataList[iThread*3+2], the value stored in the pointer is just an enum, so we it can fit in a long.

Changed 5 years ago by warmerdam

  • status changed from new to closed
  • resolution set to fixed
  • component changed from default to ConfigBuild

Based on Even's analysis and my review it seems like the issues are addressed and we can close this ticket. I'll go ahead and do so. Just reopen if there is still a pending issue.

Note: See TracTickets for help on using tickets.