dsprenkels / randombytes

A portable C library for generating cypto-secure random bytes
MIT License
96 stars 37 forks source link

Add more macro guarding for Windows #44

Open dsprenkels opened 1 year ago

dsprenkels commented 1 year ago

In #43, @raidenluikang requests that more macro guarding should be added on windows.

And add more macro for detection Windows, for ex., boost predef uses these macros:

_WIN32
_WIN64
__WIN32__
__TOS_WIN__
__WINDOWS__

@raidenluikang Can you elaborate why this is necessary? Is the current state broken in any way?

dsprenkels commented 1 year ago

Note: I think that my original code was based on https://stackoverflow.com/q/2989810/5207081

dsprenkels commented 1 year ago

Note 2: The current active documentation from Microsoft still specifies that

_WIN32 Defined as 1 when the compilation target is 32-bit ARM, 64-bit ARM, x86, or x64. Otherwise, undefined.

Reference: https://learn.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=msvc-170

The current code conforms completely to the stack. Unless the current state of the code is in some way broken, I don't really see a reason to update it?

raidenluikang commented 1 year ago

To be honest, I myself have not yet figured out this issue. But, I found this reference https://sourceforge.net/p/predef/wiki/OperatingSystems/


Identification | _WIN16 | Defined for 16-bit environments 1
-- | -- | --
Identification | _WIN32 | Defined for both 32-bit and 64-bit environments 1
Identification | _WIN64 | Defined for 64-bit environments 1
Identification | __WIN32__ | Defined by Borland C++
Identification | __TOS_WIN__ | Defined by xlC
Identification | __WINDOWS__ | Defined by Watcom C/C++

Identification  _WIN16  Defined for 16-bit environments [1](http://msdn.microsoft.com/en-us/library/ff540443.aspx)
Identification  _WIN32  Defined for both 32-bit and 64-bit environments [1](http://msdn.microsoft.com/en-us/library/ff540443.aspx)
Identification  _WIN64  Defined for 64-bit environments [1](http://msdn.microsoft.com/en-us/library/ff540443.aspx)
Identification  __WIN32__   Defined by Borland C++
Identification  __TOS_WIN__ Defined by xlC
Identification  __WINDOWS__ Defined by Watcom C/C++

Seems, __WIN32__ , __TOS_WIN__ and __WINDOWS__ compiler specific macros on windows. They maybe needed in real code, may be not. I don't able to test it. But, I know boost predef lib uses all of this macros for detection windows target.

Maybe other macros useful for embedded windows targets)

dsprenkels commented 1 year ago

Maybe other macros useful for embedded windows targets

That is an interesting point you bring up. I have not heard of any users (yet) that use this library on embedded Windows platforms. If so, please reach out. :)

The way I see it right now is, I would happily add more guarding if it fixes an issue. However, I want to be able to test that it does actually fix an issue. I would rather not copy-paste code from another library without knowing why it is there.

Is it okay if I close this issue? If, in the future, we find a configuration on which the current header is broken, we can add the extra header.

raidenluikang commented 1 year ago

There are another point. This link https://learn.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=msvc-170 which _WIN32 macro defined all windows targets is true for MSVC compiler.

Could you test another compilers: for example some of them - mingw, Borland C++, Intel C++, Embarcadero C++ Builder, Clang, TinyC compiler, Portable C compiler ?