Skip to content

Conversation

skilau
Copy link

@skilau skilau commented Sep 6, 2025

This commit brings in 2 missing endian.h includes to support the be16toh() call

Without this patch, the following compiler errors occur:

In file included from ./common/memutils.h:17,
                 from common/authenticator/authenticator.c:31:
common/authenticator/authenticator.c: In function ‘auth_send_eapol’: common/authenticator/authenticator.c:447:68: warning: implicit declaration of function ‘be16toh’ [-Wimplicit-function-declaration]
  447 |               val_to_str(hdr->packet_type, eapol_frames, "[UNK]"), be16toh(hdr->packet_body_length));
      |                                                                    ^~~~~~~

common/authenticator/authenticator_eap.c: In function ‘auth_eap_recv’: common/authenticator/authenticator_eap.c:325:9: warning: implicit declaration of function ‘be16toh’ [-Wimplicit-function-declaration]
  325 |     if (be16toh(eap->length) > buf_len) {
      |         ^~~~~~~

Per man endian(3), the be16toh() #define is in <endian.h>

This commit brings in 2 missing endian.h includes to support the be16toh() call

Without this patch, the following compiler errors occur:

In file included from ./common/memutils.h:17,
                 from common/authenticator/authenticator.c:31:
common/authenticator/authenticator.c: In function ‘auth_send_eapol’:
common/authenticator/authenticator.c:447:68: warning: implicit declaration of function ‘be16toh’ [-Wimplicit-function-declaration]
  447 |               val_to_str(hdr->packet_type, eapol_frames, "[UNK]"), be16toh(hdr->packet_body_length));
      |                                                                    ^~~~~~~

common/authenticator/authenticator_eap.c: In function ‘auth_eap_recv’:
common/authenticator/authenticator_eap.c:325:9: warning: implicit declaration of function ‘be16toh’ [-Wimplicit-function-declaration]
  325 |     if (be16toh(eap->length) > buf_len) {
      |         ^~~~~~~

Per man endian(3), the be16toh() #define is in <endian.h>
@MathisMARION
Copy link
Collaborator

Hello, this is the second time that we seem to have triggered this warning on your end without being detected on our side. I want to understand why this happens, have you made any modification to the sources?

For example, under normal circumstances, authenticator.c should include endian.h at least through this include chain:

#include "authenticator.h"
  #include "common/eapol.h"
    #include "common/endian.h"
       #include <endian.h>

I am suspecting some name conflict between our local common/endian.h and the system endian.h. Include path resolution can definitely have some compiler/platform-specific quirks. Can you try to rename common/endian.h and see if it fixes the issue on your side? Can you also share your compiler information?

@skilau
Copy link
Author

skilau commented Sep 8, 2025

Hi @MathisMARION,
Thanks for responding so quickly!

Looking at the current sources for common/endian.h, I do not see an include of the system <endian.h> in the file.

As a test, I went ahead and added:

#include <endian.h>

to the "common/endian.h" file below the include of <stdint.h>

And then removed my merge request patch here.
It resolves the issue as well.

As for modifications to the source, we just make extremely minor changes, (adding ubus support instead of dbus), as well as just cross-compiling the sources for ARM on a cross compiler package based on a musleabi toolchain.

@MathisMARION
Copy link
Collaborator

You are correct, I got confused, common/endian.h does not include the system header.

On my host (Debian testing), the system endian.h is actually included through sys/types.h. However it is a wrong assumption from us to expect this from all platforms. We will look into a way to detect this header dependency issue.

@MathisMARION
Copy link
Collaborator

Out of curiosity, what version of libc are you using?

@skilau
Copy link
Author

skilau commented Sep 11, 2025

Hi @MathisMARION : I think this is what you asking for? (Taken from our ARM product):

# ldd
musl libc (arm)
Version 1.2.5
Dynamic Program Loader
Usage: ldd [options] [--] pathname

@MathisMARION
Copy link
Collaborator

MathisMARION commented Sep 12, 2025

Thank you. We managed to reproduce your issue by building on Alpine which also uses musl libc. We will add a CI job to compile against musl to prevent any future issues. Your patch has been applied to our internal development and will be released as part of v2.7. Thank you again for the report.

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