0003611: <sys/types.h> include required for upnpnat/upnpnat.cpp

Notes
(0020472) Pol M (reporter) 2019-04-03 14:09	I'm gonna take a look :)

(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

(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.

(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

(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.

(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 clone'https://Pol_M@bitbucket.org/Pol_M/zandronum [^]' hg update addr-0003611

(0020675) Torr Samaho (administrator) 2019-05-19 19:36	Thanks, I added your patch.

(0020723) Dusk (developer) 2019-06-04 20:20	Looks like this has been fixed by now

This issue is already marked as resolved. If you feel that is not the case, please reopen it and explain why.
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
2019-06-04 20:20	Dusk	Note Added: 0020723
2019-06-04 20:20	Dusk	Status	new => resolved
2019-06-04 20:20	Dusk	Resolution	open => fixed
2019-06-04 20:20	Dusk	Assigned To	=> Torr Samaho

Relationships