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

Definition of ssize_t in FmqDefines.h clashes with definition in osconfig.h of DCMTK #178

Open
helmut-steiner opened this issue May 24, 2024 · 8 comments

Comments

@helmut-steiner
Copy link
Contributor

osconfig.h in DCMTK (a widely used DICOM Toolkit) already defines ssize_t as long:

/* Define `ssize_t' to `long' if <sys/types.h> does not define. */
#define HAVE_NO_TYPEDEF_SSIZE_T
#ifdef HAVE_NO_TYPEDEF_SSIZE_T
#define ssize_t long
#endif

When used in combination with finalmq the following error occurs in FmqDefines.h (Visual Studio 2022, SDK 19041):
image

@helmut-steiner
Copy link
Contributor Author

helmut-steiner commented May 24, 2024

Is it possible to use long instead of int64_t?
Then the simple solution would be to include the following wrapper around the using line:

#if !defined(ssize_t)
using ssize_t = std::int64_t;
#endif

Edit:
In MSVC long is 4 bytes even on 64-bit operating systems. So only 32bit. To be equal it would need to be long long.
See https://learn.microsoft.com/en-us/cpp/cpp/fundamental-types-cpp?view=msvc-170 for MSVC
and https://en.cppreference.com/w/cpp/types/integer for standard C++
So the question is, does it need to be 64bit or is 32bit sufficient?

@beckdave
Copy link
Collaborator

beckdave commented May 24, 2024

Actually, ssize_t depends on the platform.
For 64 bit platform ssize_t has 64 size. For 32 bit platform, it has 32 bit.
There should be even an ifdef for the different platforms.
_WIN64 can be used for the ifdef

@helmut-steiner
Copy link
Contributor Author

helmut-steiner commented May 24, 2024

Yes, unfortunately different libraries define ssize_t differently. This makes compiling and linking them in one project really hard.
OpenSSL does define their own ossl_ssize_t in e_os2.h e.g.:

# ifndef ossl_ssize_t
#  define ossl_ssize_t ssize_t
#  if defined(SSIZE_MAX)
#   define OSSL_SSIZE_MAX SSIZE_MAX
#  elif defined(_POSIX_SSIZE_MAX)
#   define OSSL_SSIZE_MAX _POSIX_SSIZE_MAX
#  else
#   define OSSL_SSIZE_MAX ((ssize_t)(SIZE_MAX>>1))
#  endif
# endif

As DCMTK is included first it sets the stage in our project. So ssize_t is long. finalmq library by itself compiles with int64_t. If I try to link both libraries and got some code of finalmq included the code is built with long but in finalmq it has a different type thus linking the built code with the library throws many linking errors.
I had to change the FmqDefines.h to

#if defined(WIN32) || defined(__MINGW32__)
#include <winsock2.h>
#if !defined(ssize_t)
using ssize_t = long;
#endif
#define strtof32 strtof
#define strtof64 strtod
#define MSG_MORE MSG_PARTIAL
#else
#include <sys/types.h>
using SOCKET = int;
namespace finalmq
{
static const SOCKET INVALID_SOCKET = -1;
} // namespace finalmq
#endif

and recompile everything. Afterwards compiling and linking both libraries work. Maybe a solution like in OpenSSL with another define works to keep things separated. What do you think?

@beckdave
Copy link
Collaborator

Do you compile for 32 bit platform or 64?

@helmut-steiner
Copy link
Contributor Author

Both

@beckdave
Copy link
Collaborator

This is the definition for size_t in the windows header vcruntime.h

// Definitions of common types
#ifdef _WIN64
typedef unsigned __int64 size_t;
typedef __int64 ptrdiff_t;
typedef __int64 intptr_t;
#else
typedef unsigned int size_t;
typedef int ptrdiff_t;
typedef int intptr_t;
#endif

So, size_t is different on the platforms. This should also be for ssize_t.

I would define ssize_t in finalmq as:

#if !defined(ssize_t)
#if defined(_WIN64)
using ssize_t = std::int64_t;
#else
using ssize_t = std::int32_t;
#endif
#endif


I think, DCMTK should also fix it the same way.

But in case, they will not fix it, we could do the following solution:

#if !defined(ssize_t)
#if defined(_WIN64) && !defined(FINALMQ_WIN64_SSIZE_AS_32BIT)
using ssize_t = std::int64_t;
#else
using ssize_t = std::int32_t;
#endif
#endif

I would also define FINALMQ_WIN64_SSIZE_AS_32BIT as an cmake option. This will solve your problem.

@helmut-steiner
Copy link
Contributor Author

Perfect. I will have a look into the DCMTK repository if this has been already changed in the dev branch and contact the authors if it hasn't.

@helmut-steiner
Copy link
Contributor Author

I got the response of the DCMTK team. They added it to their issue tracker:
https://support.dcmtk.org/redmine/issues/1131

They have a valid point: The above fix is only for WIN64. Other 64bit systems that might have not defined SSIZE_T will still use the wrong type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants