MantisBT - Doomseeker
View Issue Details
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
Additional Information
TagsNo tags attached.
Relationships
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2020-06-07 01:44WubTheCaptainNew Issue
2020-06-07 01:47WubTheCaptainNote Added: 0021358
2020-06-07 01:48WubTheCaptainNote Edited: 0021358bug_revision_view_page.php?bugnote_id=21358#r13107
2020-06-07 01:49WubTheCaptainSummaryC headers are included without extern "C" => C headers are included from C++ source files without extern "C"
2020-06-07 01:50WubTheCaptainNote Edited: 0021358bug_revision_view_page.php?bugnote_id=21358#r13108
2020-06-07 01:52WubTheCaptainNote Edited: 0021358bug_revision_view_page.php?bugnote_id=21358#r13109
2020-06-07 01:53WubTheCaptainNote Edited: 0021358bug_revision_view_page.php?bugnote_id=21358#r13110
2020-06-07 01:57WubTheCaptainNote Added: 0021359
2020-06-07 02:00WubTheCaptainSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=13112#r13112
2020-06-07 02:00WubTheCaptainSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=13113#r13113
2020-06-07 02:03WubTheCaptainNote Edited: 0021358bug_revision_view_page.php?bugnote_id=21358#r13114
2020-06-07 02:14WubTheCaptainSummaryC 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:14WubTheCaptainNote Edited: 0021358bug_revision_view_page.php?bugnote_id=21358#r13115
2020-06-07 02:18WubTheCaptainNote Added: 0021360
2020-06-07 02:21WubTheCaptainNote Edited: 0021360bug_revision_view_page.php?bugnote_id=21360#r13117
2020-06-07 03:43WubTheCaptainPrioritynone => low
2020-06-07 07:37WubTheCaptainNote Added: 0021368
2020-06-07 07:38WubTheCaptainNote Edited: 0021368bug_revision_view_page.php?bugnote_id=21368#r13138
2020-06-07 21:17Pol MNote Added: 0021393
2020-06-07 21:17Pol MStatusnew => feedback
2020-06-07 23:01WubTheCaptainNote Added: 0021399
2020-06-07 23:01WubTheCaptainStatusfeedback => new
2020-06-07 23:02WubTheCaptainNote Edited: 0021399bug_revision_view_page.php?bugnote_id=21399#r13154
2020-06-08 00:50Blzut3Note Added: 0021402
2020-06-08 00:50Blzut3Statusnew => resolved
2020-06-08 00:50Blzut3Resolutionopen => won't fix
2020-06-08 00:50Blzut3Assigned To => Blzut3
2020-06-08 12:34Pol MNote Added: 0021431
2021-08-07 16:52Blzut3Statusresolved => closed

Notes
(0021358)
WubTheCaptain   
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).
(0021359)
WubTheCaptain   
2020-06-07 01:57   
(assume "foo.h" would be a C system library <foo.h> in the above note)
(0021360)
WubTheCaptain   
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.
(0021368)
WubTheCaptain   
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.)
(0021393)
Pol M   
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) :)
(0021399)
WubTheCaptain   
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.
(0021402)
Blzut3   
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.
(0021431)
Pol M   
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 :)