Zandronum Chat @ irc.zandronum.com
#zandronum
Get the latest version: 3.0
Source Code

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0003611Zandronum[All Projects] Bugpublic2019-02-09 18:332019-05-19 19:36
Reportervoidpointer 
Assigned To 
PriorityhighSeveritymajorReproducibilityalways
StatusnewResolutionopen 
PlatformDockerOSAlpine LinuxOS Version
Product Version3.1-beta 
Target VersionFixed in Version 
Summary0003611: <sys/types.h> include required for upnpnat/upnpnat.cpp
DescriptionIn the zandronum-stable mercurial repository on bitbucket.org, the file "upnpnat/upnpnat.cpp" uses the symbol "u_long", which requires the include "sys/types.h" on linux. Please add this include directive so that code compiles on linux platforms.
Attached Files

- Relationships

-  Notes
User avatar (0020472)
Pol M (reporter)
2019-04-03 14:09

I'm gonna take a look :)
User avatar (0020645)
Pol M (reporter)
2019-05-13 17:20

So, I've checked the usage of u_long in the file you mentioned, and it seems to be included by the sys/socket.h entry, since that one includes bits/socket.h which includes sys/types.h

I'm not against adding an extra include just for the sake of clarity, but the code does compile ('make' notifies that it is building src/CMakeFiles/zdoom.dir/__/upnpnat/upnpnat.o and does not report any error, resulting in a working zandronum build). Maybe it's an issue in your side? A place for compiling reference is the wiki
User avatar (0020648)
voidpointer (reporter)
2019-05-13 19:13

Relying on transitive includes is a bad idea because most of the time, includes provided by other system headers are implementation-defined and not standardized. If there is clear standard documentation that states sys/socket.h must always include (directly or indirectly) sys/types.h, I will file a bug with my respective platform vendor. But I doubt this is the case since POSIX is an interface standard, and interfaces do not include header dependencies.

Note that your code is being compiled on other platforms that you don't test on. By directly including POSIX headers required by files that need them, instead of relying on indirect includes, you make your code more portable.

Thanks for looking into this for me.
User avatar (0020650)
Filystea (reporter)
2019-05-13 20:09

That is some dark mix of code.
Unix and windows code for network should be separated. Instead we have what we have.

Minimum is here:
http://pubs.opengroup.org/onlinepubs/009696699/basedefs/sys/types.h.html [^]

I see no u_long, so this is not something related to posix.
The only mistake that is done in that file is that headers are not in proper order. Which would be like this:

our own libs.

non standard libs

alphabetic order - system libs

alphabetic order - network libs

standard libs.

Alpine Linux is at fault here tbh
User avatar (0020651)
voidpointer (reporter)
2019-05-13 20:31

I'll admit I'm not too familiar with POSIX libraries. I did a quick google search prior to my last reply and didn't see anything about indirect includes being standardized.

I think the issue was on Alpine, because I remember initially trying to compile on that platform while trying to build a Docker Image for Zandronum Server. I since have switched to the ubuntu base image, which doesn't have the issue.
User avatar (0020652)
Pol M (reporter)
2019-05-13 21:08
edited on: 2019-05-13 21:10

Well, since an extra include won't hurt, I'll proceed to make the small change, regardless of how much of the fault is on Alpine's side. here you have your extra line. If you can confirm that it works on your machine, the ticket can be resolved.
you can test it on my repo:

hg clonehttps://Pol_M@bitbucket.org/Pol_M/zandronum [^]
hg update addr-0003611


User avatar (0020675)
Torr Samaho (administrator)
2019-05-19 19:36

Thanks, I added your patch.

Issue Community Support
Only registered users can voice their support. Click here to register, or here to log in.
Supporters: No one explicitly supports this issue yet.
Opponents: No one explicitly opposes this issue yet.

- Issue History
Date Modified Username Field Change
2019-02-09 18:33 voidpointer New Issue
2019-04-03 14:09 Pol M Note Added: 0020472
2019-05-13 17:20 Pol M Note Added: 0020645
2019-05-13 19:13 voidpointer Note Added: 0020648
2019-05-13 20:09 Filystea Note Added: 0020650
2019-05-13 20:31 voidpointer Note Added: 0020651
2019-05-13 21:08 Pol M Note Added: 0020652
2019-05-13 21:10 Pol M Note Edited: 0020652 View Revisions
2019-05-13 21:10 Pol M Note Edited: 0020652 View Revisions
2019-05-19 19:36 Torr Samaho Note Added: 0020675






Questions or other issues? Contact Us.

Links


Copyright © 2000 - 2019 MantisBT Team
Powered by Mantis Bugtracker