Zandronum Chat on our Discord Server Get the latest version: 3.2.1
Source Code
My View | View Issues | Change Log | Roadmap | Doomseeker Issue Support Ranking | Rules | My Account
  

View Issue Details [ Jump to Notes ] [ Issue History ] [ Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0003802Doomseeker[All Projects] Bugpublic2020-06-07 01:442021-08-07 16:52
ReporterWubTheCaptain 
Assigned ToBlzut3 
PrioritylowSeveritytrivialReproducibilityhave not tried
StatusclosedResolutionwon't fix 
PlatformOSOS Version
Product Version1.3.1 
Target VersionFixed in Version 
Summary0003802: C system headers are included from C++ source files without extern "C"
DescriptionSome unknown obscure old C libraries might not support C++, like so:
#ifdef __cplusplus
extern "C" {
#endif

void foo();

#ifdef __cplusplus
}
#endif

The C++ standard inherits some standard C headers, which are available as <cxxx> and <xxx.h>. The latter is deprecated in the standard, but available for compatibility.
But the C++ standard doesn't speak of non-standard C system .h headers, such as <windows.h> or <unistd.h> (from POSIX.1 operating system, based on C standard). There is no C++ equivalent (no <cunistd> in C++ STL or POSIX). So should they use extern "C" in the program? The C++ standard says extern "C++" is assumed by default.
Doomseeker doesn't wrap these C headers in extern "C".
Steps To ReproduceTwo examples from Doomseeker:
Quote from src/core/main.cpp
#ifdef Q_OS_OPENBSD
#include <unistd.h>
#endif

Quote from src/core/main.cpp
#ifdef USE_WINMAIN_AS_ENTRY_POINT
#include <windows.h>
/* ... */
#else
/* ... */
#endif
Attached Files

- Relationships

-  Notes
User avatar (0021358)
WubTheCaptain (reporter)
2020-06-07 01:47
edited on: 2020-06-07 02:14

 Honestly this is so trivial because every modern C library I looked at (glibc-2.30, musl-1.2.0 (git HEAD, actually), dietlibc-0.34) implements standard and non-standard headers with extern "C" via #import <sys/cdefs.h> (with __BEGIN_DECLS and __END_DECLS macro). So I didn't bother to fix this immediately without feedback / resolution first.
This issue exists because of those... obscure C libraries that may or may not exist in practice anymore. But I'm pedantic.
Imo, ignoring what those modern C standard/system libraries do, I think it's better for libraries and programs to do:
/* foo.h */
void foo();

/* foo.cpp */
extern "C" {
  #include "foo.h"
}

int main() {
  foo();
  return 0;
}

while this is bad practice imo:
/* foo.h */
#ifdef __cplusplus
extern "C" {
#endif

void foo();

#ifdef __cplusplus
}
#endif

/* foo.cpp */
#include "foo.h"

int main() {
  foo();
  return 0;
}

Yes, the latter is cleaner for modern programs utilizing modern system libraries. But the prior is perhaps more portable (and correct).
User avatar (0021359)
WubTheCaptain (reporter)
2020-06-07 01:57

 (assume "foo.h" would be a C system library <foo.h> in the above note)
User avatar (0021360)
WubTheCaptain (reporter)
2020-06-07 02:18
edited on: 2020-06-07 02:21

 If this is "won't fix" for the code, I'd rather make a documentation note somewhere for a C++ compiler / C library requirements. We're going to remain depending on those C headers.
User avatar (0021368)
WubTheCaptain (reporter)
2020-06-07 07:37
edited on: 2020-06-07 07:38

 'https://developers.redhat.com/blog/2016/02/29/why-cstdlib-is-more-complicated-than-you-might-think/ [^]'
Suggests to look back into 1998.
Quote
(If the native C library is at least minimally C++-aware, which wasn’t always true in 1998 but is almost universally true now, then the extern "C" linkage specification shown above will be present in the C headers themselves, and so isn’t needed in the <cxxx> header.)
User avatar (0021393)
Pol M (developer)
2020-06-07 21:17

 I honestly think that most modern C programs have the extern "C" guard. If we ever encounter such an issue, we can create an "include_[library].h" header that puts the guard around the desired header. That said, I've seen <string.h> somewhere, and that can be changed without issues to cstring (same goes for similar cases) :)
User avatar (0021399)
WubTheCaptain (reporter)
2020-06-07 23:01
edited on: 2020-06-07 23:02
Quote from Pol M
That said, I've seen <string.h> somewhere, and that can be changed without issues to cstring (same goes for similar cases) :)

There is Doomseeker's own string handling header, strings.hpp. It conflicted with glibc's <string.h>. See 0003385.
Quote from src/core/strings.hpp
// Why hpp in this case? Glibc 2.26 changed string.h to include its strings.h
// so this file would shadow that header and cause errors.

The only remaining <string.h> is in LZMA (not in our code). So I'm also not sure if or what you're proposing.
User avatar (0021402)
Blzut3 (administrator)
2020-06-08 00:50

 The suggestion goes against the coding practices of literally every other C++ program in the wild. I see why you would come to the conclusion that the program doing extern "C" instead of the library is more correct, but that's just not how the world decided to solve the problem. Probably because extern "C" only solves the name mangling problem and doesn't solve other issues with using C headers like potential use of reserved key words. So the header needs to be polyglot anyway.
User avatar (0021431)
Pol M (developer)
2020-06-08 12:34
Quote from Sub

The only remaining <string.h> is in LZMA (not in our code). So I'm also not sure if or what you're proposing.

True, my bad :)

Issue Community Support
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
2020-06-07 01:44 WubTheCaptain New Issue
2020-06-07 01:47 WubTheCaptain Note Added: 0021358
2020-06-07 01:48 WubTheCaptain Note Edited: 0021358 View Revisions
2020-06-07 01:49 WubTheCaptain Summary C headers are included without extern "C" => C headers are included from C++ source files without extern "C"
2020-06-07 01:50 WubTheCaptain Note Edited: 0021358 View Revisions
2020-06-07 01:52 WubTheCaptain Note Edited: 0021358 View Revisions
2020-06-07 01:53 WubTheCaptain Note Edited: 0021358 View Revisions
2020-06-07 01:57 WubTheCaptain Note Added: 0021359
2020-06-07 02:00 WubTheCaptain Steps to Reproduce Updated View Revisions
2020-06-07 02:00 WubTheCaptain Steps to Reproduce Updated View Revisions
2020-06-07 02:03 WubTheCaptain Note Edited: 0021358 View Revisions
2020-06-07 02:14 WubTheCaptain Summary C headers are included from C++ source files without extern "C" => C system headers are included from C++ source files without extern "C"
2020-06-07 02:14 WubTheCaptain Note Edited: 0021358 View Revisions
2020-06-07 02:18 WubTheCaptain Note Added: 0021360
2020-06-07 02:21 WubTheCaptain Note Edited: 0021360 View Revisions
2020-06-07 03:43 WubTheCaptain Priority none => low
2020-06-07 07:37 WubTheCaptain Note Added: 0021368
2020-06-07 07:38 WubTheCaptain Note Edited: 0021368 View Revisions
2020-06-07 21:17 Pol M Note Added: 0021393
2020-06-07 21:17 Pol M Status new => feedback
2020-06-07 23:01 WubTheCaptain Note Added: 0021399
2020-06-07 23:01 WubTheCaptain Status feedback => new
2020-06-07 23:02 WubTheCaptain Note Edited: 0021399 View Revisions
2020-06-08 00:50 Blzut3 Note Added: 0021402
2020-06-08 00:50 Blzut3 Status new => resolved
2020-06-08 00:50 Blzut3 Resolution open => won't fix
2020-06-08 00:50 Blzut3 Assigned To => Blzut3
2020-06-08 12:34 Pol M Note Added: 0021431
2021-08-07 16:52 Blzut3 Status resolved => closed




Questions or other issues? Contact Us.

Links

Copyright © 2000 - 2025 MantisBT Team
Powered by Mantis Bugtracker