Get rid of the need for -fcommon

[l3u]

Hi all :-)

On Gentoo's Bugzilla, I got a bug report about clang not being able to build vcontrold. With commit b68ba0b, I added a workaround for this by adding -fcommon not only for GCC >= 10, but also for clang.

I don't know much about C (more about C++), and I never worked with anything older than C++11. So I did some research about -fcommon. On GCC's Bugzilla, in the bug that finally lead to the removal of -fcommon in GCC 10, you can read:

The use of a "common" section to match up tentative object definitions in different translation
units in a program is contrary to the C standards (section 6.9p5 in C99 and C11, section 6.7 in
C90), gives a high risk of error, may lead to poorer quality code, and provides no benefits
except backwards compatibility with ancient code.

Reading this, I think that instead of testing for compiler versions and adding -fcommon if needed, we should better fix the root issue and get rid of the whole -fcommon thing (nobody wants to have "ancient" code, does he?!).

Both GCC and clang fail with (almost) the same error when invoked without -fcommon here:

FAILED: vclient 
: && /usr/bin/cc  -rdynamic CMakeFiles/vclient.dir/src/common.c.o
CMakeFiles/vclient.dir/src/socket.c.o CMakeFiles/vclient.dir/src/io.c.o
CMakeFiles/vclient.dir/src/client.c.o CMakeFiles/vclient.dir/src/vclient.c.o -o vclient   && :
/usr/lib/gcc/x86_64-pc-linux-gnu/11.2.0/../../../../x86_64-pc-linux-gnu/bin/ld:
CMakeFiles/vclient.dir/src/vclient.c.o:(.bss+0x0): multiple definition of `TxRx';
CMakeFiles/vclient.dir/src/client.c.o:(.bss+0x0): first defined here
collect2: error: ld returned 1 exit status

Alas, I couldn't figure out why this happens … there's only one single line containing TxRx in the whole sources, (if I understood it correctly) creating an instance of the txRx struct as TxRx in client.h:

struct txRx {
    char *cmd;
    float result;
    char *err;
    char *raw;
    time_t timestamp;
    trPtr next;
} TxRx;

This seems to be beyond my (non-existing) C knowledge ;-) So: Can anybody fix this so that we can get rid of -fcommon?! This would definitely be good for the code base.

Thanks for all help and/or explanation!

Cheers, Tobias

[speters]

In client.h is a definition of TxRx, so it gets instantiated. A typedef fixes this to make this just a declaration.

What I don't understand is why it seems to get included more than once despite the #ifndef CLIENT_H include guard. #pragma once might be a more modern, but this does not fix it either.

The simple edits are in branch fix_fno_common_typedef. It's untested, as I don't run vcontrold any more, so please check before merging.

[l3u]

Thanks for the fix! Makes it compile both with GCC and Clang, without the need for -fcommon. I'll merge it and fix CMakeLists.txt.

[l3u]

PS: Tested on my Raspberry Pi 1, built with GCC 11.2.0. Works without a problem.

[l3u]

PPS:

What I don't understand is why it seems to get included more than once despite the #ifndef CLIENT_H include guard.

As far as I can grasp it, the header actually isn't included more than once in one compilation unit. But it's added once in different units that are finally linked together, and that's where the collision happened.

Well, however, the problem is gone :-)