Fiwix does not build with newest gcc cross-compiler

[rick-masters]

Following the instructions here, I am trying to build Fiwix with the newest gcc cross-compiler but it fails with many errors like this:

ld: kernel/init.o:(.bss+0x0): multiple definition of `vcbuf'; kernel/gdt.o:(.bss+0x30): first defined here

Starting in version 10, gcc has adopted -fno-common as a default option. See here for details.

There is quite a bit of discussion on whether common variables are allowed by "the standards" and it appears to me that supporting them is "optional" for a C compiler. I have my own opinion on the use of common variables which I'll share if asked but I'll defer to your judgment on how to resolve this.

If you think gcc is being too strict, the "-fcommon" option can be added to the CFLAGS and the problem is resolved.

Alternatively, the variables can be declared once in .c files and declared extern in headers. Also, the "-fno-common" flag could be added to CFLAGS to ensure the problem is flagged in the future for older versions of gcc. This is substantially more work as there are over a dozen variables to address.

I'm happy to submit submit a PR for either option.

[mikaku]

I'm using a GCC 4.7.4 cross-compiler to develop Fiwix under my Fedora workstation, and I don't see any compiler error or an ugly warning. I like to think that Fiwix has a polished code and I work on this direction.

According the Fiwix code this is the only line where this variable is declared:

https://github.com/mikaku/Fiwix/blob/24a59814001999ea27ef48fdfc3b50fdcf7032d9/include/fiwix/console.h#L107

Oh!, perhaps this declaration is triggering the warning?

https://github.com/mikaku/Fiwix/blob/24a59814001999ea27ef48fdfc3b50fdcf7032d9/mm/memory.c#L423

This is part of an ugly piece of code flagged as FIXME, pending to be fixed.

I like the idea to introduce the declaration of extern in order so solve all these issues on newer GCC versions, this will keep the kernel code as standard as possible and avoid GCCisms with specific compiler options.

Do you agree?

[rick-masters]

If you compile with -fno-common you should see the error.

I agree that using extern is the right solution. The idea is that the variable should be defined only in one C file and the rest of the files use extern to refer to it.

The memory.c reference is not the problem with vcbuf. The issue is that console.h is included in many C files directly and indirectly from other headers. So, it ends up being defined (without extern) in multiple C files. It should be declared extern in console.h and defined without extern at the top of drivers/char/console.c or some other C file. (Note that console.c will also see the extern declaration from console.h before it defines it but that redundancy is normal.)

The distinction between declaration and definition is described in detail here

There are around 18 other variables with the same problem. I can provide a PR that fixes all of them. Would you like to also have -fno-common added to the Makefile to flag this in the future?

[mikaku]

I can provide a PR that fixes all of them. Would you like to also have -fno-common added to the Makefile to flag this in the future?

If you provide a PR I'll merge it.

Is good to have the option -fno-common in the Makefile to help us to detect these situations, but I'm not sure if it should go in the main make or we should create a special make strict separately with this option.

[rick-masters]

PR submitted. I did put that option in the Makefile because it is, in my humble opinion, a common coding standard for C for polished code and it will help keep the code up to snuff, but I can remove it if you really want that.

[mikaku]

If it's a common coding standard in C then it's fine for me. Overall though, I prefer to stay in old standards not new.

[mikaku]

I have a problem merging this PR as it touches a lot of files that I have already changed here but not yet submitted.

[mikaku]

I finally managed to merge it. Thank you.