Conflict caused by typedef SSIZE_T ssize_t;

[p0358]

Description:
There's this line in rapidcsv:

typedef SSIZE_T ssize_t;

However some codebases that include for example nghttp2 may have already set up ssize_t as a macro on project level.

The solution is to check #ifndef ssize_t before trying to define ssize_t, otherwise we'll get an error that we're trying to redefine a base type.

How to reproduce it:

#define ssize_t=__int64
#define _SSIZE_T_DEFINED=1

Environment:

• Version: v8.69
• OS / distro: Windows 10 with nghttp2 library included

[d99kris]

Hi @p0358 - thanks for reporting the bug. It's an interesting problem! While checking #ifndef ssize_t will work in this particular scenario, I believe it will not work if another header has typedef'ed ssize_t to something else.

Actually ssize_t is mostly used internally in rapidcsv and not exposed much in the public API. I'm wondering if maybe it makes sense to replace it with int64_t to avoid the problem altogether. I'll need to do some research and get back.

[sven-molkenstruck]

@d99kris If you want to keep your typedef, I suggest to move it into your rapidcsv namespace. That should avoid conflicts with potential typedefs in other headers.

[d99kris]

Thanks for the suggestion @Sven-HP - that is true! But I think I'm more inclined to drop the typedef and simply use a standard integer type. I'd prefer if portable apps would not need stuff like rapidcsv::ssize_t x = doc.GetColumnIdx("name"); when they do interact with an API exposing the type.

I've been thinking about which type to use, and I think regular int will do. For CSV files with over 2 billion rows or columns it probably becomes impractical to use rapidcsv anyway (as rapidcsv keeps the entire parsed CSV in memory so the memory usage would be... quite high..). int64_t would be a safer choice, but it has performance impact on 32-bit environments.

[d99kris]

I've created a PR #139 for switching ssize_t usage to int. Feel free to provide feedback on the change in the review. If there are no major concerns I will proceed to merge it next weekend probably.

[d99kris]

Thanks for checking the PR @Sven-HP!

This issue should be fixed now. Please let me know if you still encounter any issues @p0358. Thanks.