search

thom311 / libnl

Netlink Library Suite
GNU Lesser General Public License v2.1
419 stars 311 forks source link

Fix some errors, when using with C++ #380

Closed ievenbach closed 4 months ago

ievenbach commented 4 months ago
  1. "restrict" keyword does not exist in C++
  2. Use explicit cast from malloc to parameter type used by _nl_inet_ntop

Note: this isn't enough to use pure C++. E.g. some macros use typeof() operator, which is GCC extension.

thom311 commented 4 months ago

The public headers of libnl need to be valid C++. However, for building the library itself, I think you really should use one of the supported C compiler. Basically it's either gcc+clang, which provide invaluable additions to plain C (typeof, cleanup attribute).

Why do you really want to build with a C++ compiler? Which one? Is that a good idea (to do, and to support)?

One problem is that there are no unit tests that test building with a C++ compiler. Although, it would be very useful, to test at least whether the public headers compile with C++ (and link correctly with extern "C"). Here you change something for how to build the library itself, something which maybe doesn't make sense to support and which is not covered by tests. It's bound to break again.

btw, some source files are formatted with clang-format. See the github action test that fails. You can also run ./tools/clang-format.sh or ./tools/clang-format-container.sh, the former will require a suitable version of clang-format on your system, the latter will run a podman container. Or just adjust manually.

ievenbach commented 4 months ago

The library itself is being built with C (clang). I had to put it into GCC mode, which is annoying, but OK. The C++ part is the app I'm using.

ievenbach commented 4 months ago

Run clang-format

thom311 commented 4 months ago

The library itself is being built with C (clang). I had to put it into GCC mode, which is annoying, but OK. The C++ part is the app I'm using.

I still don't understand what problem this patch solves.

The file you modify is an internal file. It's only used for building libnl library itself.

On the other hand, throughout the code base there are plenty implicit void* casts. So even with those two changes, you still cannot compile the library with C++.

What exactly are you doing? And how does this patch make that work?

ievenbach commented 4 months ago

Looks like I just didn't realize it's private header. I got everything to compile when I turned on GNU extensions for typeof(), and plain forgot about it.