CESNET / ipfixcol

IPFIXcol is an implementation of an IPFIX (RFC 7011) collector
Other
64 stars 37 forks source link

C/C++ supported standards #195

Closed SecDorks-TorchSis closed 6 years ago

SecDorks-TorchSis commented 6 years ago

Hello!

I have some minor changes for some plugins. I was looking in the coding guidelines and there is no mention on the coding standards used in the codebase. I would assume it is C89 and C++98, but in fastbit plugin there is already C++11 code and some parts of the C code use C99 features (but these are not used extensively).

Is it fine for new code to use these two or earlier standards? This would mean that old platforms won't be supported any more (at least not in their default setups).

thorgrin commented 6 years ago

Hi, we use LBR_SET_CXXSTD([gnu++11]) macro in configure.ac to say that the plugin require the gnu++11 standard. The macro can be used to ensure other standards, but we use the gnu++11 quite often. When no parameter is given, the highest supported standard is used (up to gnu++11, it should probably be updated).

Since the base requires gnu++11, there is no reason to limit the plugins to use older standards. Just be sure to check and modify the use of LBR_SET_CXXSTD to explicitly demand the standard when you update the plugin.

SecDorks-TorchSis commented 6 years ago

Hello!

Thank you @thorgrin for your response! The fact is I was compiling using the GNU11 version of the language, but I thought it was because of my local, development setup. There is no mentioned standard in the guidelines, and that's why I got confused.

GNU11 as the C++ standard is a very sensible option, as we currently do have code that depends on C++11 features and it would be sad to degrade the quality of the code, only to support platforms that obviously are not using IPFIXcol. On the contrary, I now feel free to use more C++11 features that will simplify and greatly improve the quality of the C++ code.

But, what about the C standard? Again, this is not specified and on my system it is set on GNU99. Most of the C code is written on strict ANSI C89 style, but on some, rare, occasions, new features from C99 are being used. Shall we continue assume we are compiling using a C99 compiler?

As a final note, I think it should be better to restrict the language standards to strict c++11 and c99: I didn't notice any exotic GNU-specific feature in use, and this option would allow us to build IPFIXcol on other than GNU platforms, with LLVM coming first into my mind.

thorgrin commented 6 years ago

Hello,

We have started with the old ANSI C89 at first, but now we use C99 freely and the code would not compile with ANSI C89 (there are many for (int i = 0; ...) construction for example). So I think it is safe to declare that C99 and C++11 features may be used at all times.

As for the GNU extensions, our systems (various Linux distributions) support it, and we did not see any reason not to use them. I've tried to compile base just using ./configure CFLAGS='-std=c99' CXXFLAGS='-std=c++11' and it failed pretty quickly. It seems that at least std=gnu99 is needed for IPFIXcol. If you need the IPFIXcol to run on non-GNU platform, we can try to remove such limitations, however, I'd not like to spend much time on this feature before it is seriously needed.

I've just updated the coding_style (devel branch), please let me know if it is clear enough.

Thanks

SecDorks-TorchSis commented 6 years ago

Hello!

I'm perfectly fine with the standards mentioned in the documentation. It is not worthy, for the moment, to try to adjust existing code into strict C99 standard mode.

It is very useful that these standards are mentioned in the coding style document. We can now align our code to the required versions!

Thank you, once again, for clarifying the issue.