Fix code style

[BenBE]

Some code style updates …

[Explorer09]

Commit b97ac54 introduced build errors in CI. The problem of that is a bit tricky.

In the two files linux/LinuxProcessTable.c and linux/CGroupUtils.c there are calls to String_strchrnul() which would depend on strchrnul() when it's found at configure time. However, glibc strchrnul() is accessible only if _GNU_SOURCE is defined, which requires the #include "config.h" line before any other header.

In other words, because of the way String_strchrnul() is defined in XUtils.h, every source file that calls String_strchrnul() would need #include "config.h" line in addition to #include "XUtils.h". This becomes a bit inconvenient as XUtils.h would not look "standalone". I'm not sure what to do with the problem yet.

EDIT: There are two solutions.

1. Just add the necessary #include "config.h" lines and ignore the "feature test macros" dependency hell.
2. Make String_strchrnul() an extern function in XUtils.h, and define the function body in XUtils.c. This would rely on LTO to inline the function calls properly.

[fasterit]

#ifndef _GNU_SOURCE
# define _GNU_SOURCE 1
#endif

inside the #ifdef HAVE_STRCHRNUL block?

[Explorer09]

@fasterit Nope. That won't work. The #define _GNU_SOURCE line has to be before any include line of the standard header (<string.h> for this particular case), or else the header guard of glibc's <string.h> would prevent the strchrnul() from making available.

The two "solutions" I mentioned above are the proper ways to do it. If it's me I would pick option 1, as config.h is required for many .c files already in htop. It won't be a difficult policy to mandate every .c file include config.h.

[fasterit]

Wtf.

I'd favor your option 1 then, too. The lesser evil.

Thank you very much for the analysis @Explorer09 !

/DLange

[Explorer09]

@fasterit In addition I would propose to remove the #include "config.h" line from XUtils.h and replace with the following:

#ifndef PACKAGE_NAME
#error "Must have #include \"config.h\" line at the top of the .c file, or else some XUtils.h functions might break."
#endif

The error message is self-explanatory.

[fasterit]

Seems to work for all but the PCP build...

./XUtils.h:13:2: error: #error "Must have #include \"config.h\" line at the top of the file that includes these XUtils helper functions"
   13 | #error "Must have #include \"config.h\" line at the top of the file that includes these XUtils helper functions"
      |  ^~~~

While it is included, of course.

[BenBE]

What a fitting kind of issue to have as PR 1337 … ;-)

[BenBE]

@fasterit The PCP failure comes from the brain-dead design of PCP that requires you to undefine PACKAGE_NAME because they are re-using that identifier in their own headers. I'll have a look to update the check to use an HTOP_ specific define instead.

[Explorer09]

Didn't see any problems now with this PR. Just a final note.

From the commit description:

Also add a #error stanza to XUtils.h in case somebody forgets the beautiful mess GNU forces on us.

It's not just GNU, but all feature test macros coming from POSIX and any of its extensions are required to be defined this way. This "feature" is kind of annoying for developing in the C language (unlike C++, which comes with namespaces to clean up the duplicated APIs and symbol name conflicts). I hate it too, but we have to live with it.

Feature test macros was one of the reasons GNU autotools introduced config.h, although it only solves half of the problem. The use of config.h is incompatible with the IWYU's coding paradigm (that's the other half of the problem).

[BenBE]

One more thing. Because of the feature test macro mess. I'm now thinking that the #include "config.h" lines might be removed from all "module" headers of htop. Interestingly, not all htop header files have this include line. I wonder how and why IWYU suggested including config.h for these headers in the first place (specifically, what tokens or symbols made IWYU suggest the config.h)

I moved the includes of config.h from all header files (except Provide*.h) to the corresponding C module source. Fallout was about 6 more files that needed config.h as their first include. Hope I caught all by now …

[Explorer09]

@fasterit Just curious. Would the current main branch (after the merge of this PR) cause any build error with your PCP configuration? While the CI server does build it fine, I think you may test building it with your environment just to be sure.

And there are two .c files in htop that have the #undef PACKAGE_* lines as workarounds. Are the workaround lines (like the following) still needed?

#undef PACKAGE_NAME
#undef PACKAGE_TARNAME
#undef PACKAGE_VERSION
#undef PACKAGE_STRING
#undef PACKAGE_BUGREPORT
#undef PACKAGE_URL
#include <pcp/pmapi.h>
#undef PACKAGE_NAME
#undef PACKAGE_TARNAME
#undef PACKAGE_VERSION
#undef PACKAGE_STRING
#undef PACKAGE_BUGREPORT
#undef PACKAGE_URL

[fasterit]

I think you want @natoscott for this @Explorer09.

[BenBE]

Technically they are no longer needed once the variables for the corresponding defines in Makefile.am/configure.ac are renamed. But that's not an issue of this PR, but a separate issue for @natoscott to take care of.

[Explorer09]

@BenBE The default C macro names like PACKAGE_NAME might cause conflicts if htop's "config.h" and PCP's internal "config.h" are included together. The computer might produce a "macro redefinition" warning. That is what I was talking about.

AFAIK, there is no way for autoconf to not predefine these default PACKAGE_* C macros in "config.h".

(And just for the info, the C macros PACKAGE and VERSION, are predefined by automake. They are not to be confused with autoconf-defined PACKAGE_NAME and PACKAGE_VERSION macros. The two automake macros can be suppressed by the no-define automake option.)

[AryanGitHub]

I wanted to know, what is the workaround for building PCP build?
I got the error and warning at the time of building,
waring
configure.ac:385: warning: pkg.m4 is absent or older than version 0.16; this 'configure' would have incomplete pkg-config support
and error as
checking for pcp/pmapi.h... no configure: error: can not find PCP header file

[natoscott]

@AryanGitHub if you want to build pcp-htop you just need to install the devel package for your distribution which includes the header file mentioned there.