Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format specifier macros for cgsize_t #754

Closed
jedbrown opened this issue Feb 23, 2024 · 2 comments
Closed

Format specifier macros for cgsize_t #754

jedbrown opened this issue Feb 23, 2024 · 2 comments

Comments

@jedbrown
Copy link

HDF5 has the following definitions, which allow reading and formatting of type hsize_t.

typedef uint64_t hsize_t;
typedef int64_t hssize_t;
#define PRIdHSIZE          PRId64
#define PRIiHSIZE          PRIi64
#define PRIoHSIZE          PRIo64
#define PRIuHSIZE          PRIu64
#define PRIxHSIZE          PRIx64
#define PRIXHSIZE          PRIX64
#define H5_SIZEOF_HSIZE_T  8
#define H5_SIZEOF_HSSIZE_T 8

CGNS has

# if CG_BUILD_64BIT
#  define CG_SIZEOF_SIZE    64
#  define CG_SIZE_DATATYPE "I8"
   typedef CG_LONG_T cgsize_t;
# else
#  define CG_SIZEOF_SIZE    32
#  define CG_SIZE_DATATYPE "I4"
   typedef int cgsize_t;
# endif

Unfortunately, CG_LONG_T can take on different values depend on whether it's on Windows. It's also incompatible with hssize_t because int64_t usually maps to long (e.g., on x86-64) instead of long long. (I'm not saying they should be compatible, but HDF5 changed how they handle these types in HDFGroup/hdf5#709 and I think that rationale also applies to CGNS.)

This is a minor headache for us in PETSc. Would you be amenable to a change similar to the HDF5 change above and adding PRIdCGSIZE (and others if someone wants them)?

Also, I'm assuming for now that 32-bit cgsize_t is important for some users. That makes it harder to package/distribute and requires libraries like ours to test both versions.

@brtnfld
Copy link
Member

brtnfld commented Feb 26, 2024

I don't see any issues with making similar changes; we already require C99.

@MicK7 MicK7 mentioned this issue Mar 3, 2024
MicK7 added a commit that referenced this issue Mar 5, 2024
* use C99 types int64_t

- define cgsize_t as a intxx_t type
- provide PRIdCGSIZE
- follow hdf5 changes
@brtnfld
Copy link
Member

brtnfld commented Apr 9, 2024

This has been fixed and should be in the next release. Thanks for the report.

@brtnfld brtnfld closed this as completed Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants