Version member naming in Common.h collides with certain macros set on some *NIX based systems

[jimkoen]

The definition of this constructor shadows macros set in <sys/types.h> on certain *nix based systems.

This happens on FreeBSD with the following compilation output indicating a macro shadow:

/home/jimkoen/github/BeamMP-Server/src/Common.cpp:295:7: error: member initializer '__major' does not name a non-static data member or base class
    : major(major)
      ^~~~~~~~~~~~
/usr/include/sys/types.h:389:18: note: expanded from macro 'major'
#define major(d)        __major(d)
                        ^~~~~~~~~~
/home/jimkoen/github/BeamMP-Server/src/Common.cpp:296:7: error: member initializer '__minor' does not name a non-static data member or base class
    , minor(minor)
      ^~~~~~~~~~~~
/usr/include/sys/types.h:395:18: note: expanded from macro 'minor'
#define minor(d)        __minor(d)
                        ^~~~~~~~~~

And this has apparently since happened before on Ubuntu, see #177 .

On FreeBSD specifically, these macros are responsible for the collision. Since this has happened two times already on entirely different platforms, I'd suggest refactoring this section entirely.

[lionkor]

minor is a valid identifier. This is a platform bug, i.e. needs a platform specific workaround. I suggest #ifdef followed by #undef of these macros after all includes.

Are you supposed to include headers which define this macro, i.e. sys/types.h, or is this just an infortunate naming coincidence where freebsd happens to have a sys/types.h header which is included in the linux code path because its different on linux?

Either way the macros are the problem, not the naming.

[lionkor]

I would suggest doing thr platform specific includes, follwed by the ifdefs and undefs, in the compatibility header or whatever its called

[jimkoen]

minor is a valid identifier. This is a platform bug, i.e. needs a platform specific workaround. I suggest #ifdef followed by #undef of these macros after all includes.

That was the workaround I used that lead to me writing this issue.

Are you supposed to include headers which define this macro, i.e. sys/types.h, or is this just an infortunate naming coincidence where freebsd happens to have a sys/types.h header which is included in the linux code path because its different on linux?

It seems this gets imported by CMake automatically? I've checked the entire project, but can't find any import of <sys/types.h>. So it's likely that this isn't our fault. This also isn't FreeBSD specific, as the person in issue #177 had the exact same issue, with the exact same error message, and the exact same workaround.

Either way the macros are the problem, not the naming.

Just saying. Pretty sure the macros won't go anywhere, and idk I feel like an undef is a rather ugly way to deal with this.

[lionkor]

Rename it to maj and min and suddenly the windows min() macro will collide. You can't win with these idiotic C headers. What's ugly is that they took a super normal word and overrode it with a global macro in a header. This is why double underscore is for reserved identifiers -- so they dont do this shit :)

I think #undef in Compatibility.h is the cleanest way to deal with these collisions. They're dirty implementations, they need a dirty fix imo

[lionkor]

My main question is what causes __BSD_VISIBLE to be set? Because that's not supposed to be set, I assume?

[lionkor]

https://freebsd-newbies.freebsd.narkive.com/cy9WtxxL/what-is-bsd-visible-for

I think we want to make sure __BSD_VISIBLE is not set on BSD as to not break POSIX (i guess lol)

[jimkoen]

This was made superfluous by fixing #160