Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from

Conversation

ievenbach
Copy link
Contributor

  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
Copy link
Owner

thom311 commented Apr 24, 2024

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
Copy link
Contributor Author

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.

1. "restrict" keyword does not exist in C++
2. Use explicit cast from malloc to parameter
   type used by _nl_inet_ntop
@ievenbach
Copy link
Contributor Author

Run clang-format

@thom311
Copy link
Owner

thom311 commented Apr 26, 2024

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
Copy link
Contributor Author

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.

@ievenbach ievenbach closed this Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants