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

Minor compilation error on armv6 (32 bits) #10

Closed
solsticedhiver opened this issue Sep 22, 2022 · 7 comments
Closed

Minor compilation error on armv6 (32 bits) #10

solsticedhiver opened this issue Sep 22, 2022 · 7 comments

Comments

@solsticedhiver
Copy link

solsticedhiver commented Sep 22, 2022

In a very specific build setup (using meson+ninja and gcc only), on armv6l, I came across that build error:

[112/132] Compiling C object subprojects/libwifi-0.0.6/examples/parse_eapol.p/parse_eapol_parse_eapol.c.o
FAILED: subprojects/libwifi-0.0.6/examples/parse_eapol.p/parse_eapol_parse_eapol.c.o 
cc -Isubprojects/libwifi-0.0.6/examples/parse_eapol.p -Isubprojects/libwifi-0.0.6/examples -I../subprojects/libwifi-0.0.6/examples -I../subprojects/libwifi-0.0.6/src -I/usr/local/include -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -g -Wall -Werror -O3 -MD -MQ subprojects/libwifi-0.0.6/examples/parse_eapol.p/parse_eapol_parse_eapol.c.o -MF subprojects/libwifi-0.0.6/examples/parse_eapol.p/parse_eapol_parse_eapol.c.o.d -o subprojects/libwifi-0.0.6/examples/parse_eapol.p/parse_eapol_parse_eapol.c.o -c ../subprojects/libwifi-0.0.6/examples/parse_eapol/parse_eapol.c
../subprojects/libwifi-0.0.6/examples/parse_eapol/parse_eapol.c: In function ‘handle_pkt’:
../subprojects/libwifi-0.0.6/examples/parse_eapol/parse_eapol.c:41:56: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘uint64_t’ {aka ‘long long unsigned int’} [-Werror=format=]
   41 |             printf("EAPOL: Key Info: Replay Counter: %lu\n", data.key_info.replay_counter);
      |                                                      ~~^     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                        |                  |
      |                                                        long unsigned int  uint64_t {aka long long unsigned int}
      |                                                      %llu
cc1: all warnings being treated as errors
[113/132] Compiling C object subprojects/libwifi-0.0.6/utils/test_misc.p/src_helpers.c.o
ninja: build stopped: subcommand failed.

This is easly fixed with that patch:

diff --git a/examples/parse_eapol/parse_eapol.c b/examples/parse_eapol/parse_eapol.c
index cf35a6f..f36685b 100644
--- a/examples/parse_eapol/parse_eapol.c
+++ b/examples/parse_eapol/parse_eapol.c
@@ -38,7 +38,7 @@ void handle_pkt(unsigned char *args, const struct pcap_pkthdr *header, const uns
             printf("EAPOL: Descriptor: %d\n", data.descriptor);
             printf("EAPOL: Key Info: Information: 0x%04x\n", data.key_info.information);
             printf("EAPOL: Key Info: Key Length: %d\n", data.key_info.key_length);
-            printf("EAPOL: Key Info: Replay Counter: %lu\n", data.key_info.replay_counter);
+            printf("EAPOL: Key Info: Replay Counter: %"PRIu64"\n", data.key_info.replay_counter);
             printf("EAPOL: Key Info: Nonce: ");
             for (size_t i = 0; i < sizeof(data.key_info.nonce); ++i) {
                 printf("%02x ", data.key_info.nonce[i]);
diff --git a/utils/src/test_parsing.c b/utils/src/test_parsing.c
index b9f9dbc..84a1bde 100644
--- a/utils/src/test_parsing.c
+++ b/utils/src/test_parsing.c
@@ -370,7 +370,7 @@ void parse_data_eapol(struct libwifi_frame frame, unsigned char *args, const str
             printf("EAPOL: Descriptor: %d\n", data.descriptor);
             printf("EAPOL: Key Info: Information: 0x%04x\n", data.key_info.information);
             printf("EAPOL: Key Info: Key Length: %d\n", data.key_info.key_length);
-            printf("EAPOL: Key Info: Replay Counter: %llu\n", data.key_info.replay_counter);
+            printf("EAPOL: Key Info: Replay Counter: %"PRIu64"\n", data.key_info.replay_counter);
             printf("EAPOL: Key Info: Nonce: ");
             for (size_t i = 0; i < sizeof(data.key_info.nonce); ++i) printf("%02x ", data.key_info.nonce[i]);

The second snippet avoids a warning

@foxtrot
Copy link
Member

foxtrot commented Oct 1, 2022

Hi

Why not use %llu instead? It's much more ubiquitous than %PRIu64 and should work fine with uint64_t's on 32 bit platforms?

On x86 (32 bit) I get a clean build on current main. What compiler version are you using?

@solsticedhiver
Copy link
Author

solsticedhiver commented Oct 2, 2022

First I would like to point out that the exammples Makefile are using clang compiler (while the rest is using gcc?). and kind of expect libwifi.h to be installed systemd wide.

I was using gcc, even for the examples. my meson build stuff was building the examples by default.
and %llu throws a warning with gcc (or something else ?). That's why I changed the 2.

%PRIu64 is less common and I don't use it or even know it well, but this is what you are supposed to use to print a uint64_t. It came with C99 and the same time than uint64_t.

But like I said, it a minor problem, I could have said very minor problem. And I understand if you close it without much thinking for it.

@foxtrot
Copy link
Member

foxtrot commented Oct 2, 2022

First I would like to point out that the exammples Makefile are using clang compiler (while the rest is using gcc?).

The example makefiles should be updated, yes. The main project uses whatever compiler is available by default.

and kind of expect libwifi.h to be installed systemd wide.

I'm not sure what you mean here? sudo make install will place the header(s) into /usr/local/include.

I was using gcc, even for the examples. my meson build stuff was building the examples by default. and %llu throws a warning with gcc, I guess. That's why I changed the 2.

Your error report is correct, but only in one place, I think. examples/parse_eapol/parse_eapol.c should be %llu. The other edit is unnecessary I think :)

@foxtrot foxtrot closed this as completed in 9f6dca8 Oct 2, 2022
@solsticedhiver
Copy link
Author

solsticedhiver commented Oct 2, 2022

Thinking about this, I was about to edit my comment.

Just to say that PRIu64 is more universal and generic and will print a uint64_t whatever its size and the architecture.

I meant that if I am not wrong, you can't compile the examples without installing the lib and its headers. I would have expect to build the examples without installing. but that is a matter of taste ?

But never mind

@foxtrot
Copy link
Member

foxtrot commented Oct 2, 2022

I meant that if I am not wrong, you can't compile the examples without installing the lib and its headers. I would have expect to build the examples without installing. but that is a matter of taste ?

Ah, I understand. I have only ever seen examples and tests use the globally installed version (such as libpcap).

@solsticedhiver
Copy link
Author

solsticedhiver commented Oct 24, 2022

You did noticed that the example fails to compile on x64 now, right?
it's because an uin64_t is an unsigned long on x64 but a unsigned long long on x32

You can't solve the problem for the 2 architectures by using %lu or %llu.
The way to go is to use PRIu64. That's why I was pushing for it.

https://stackoverflow.com/a/9225648/283067

Or you revert the commit fixing that, and ignore 32 bits architecture ;-(

And I got that error in the first place, because I was building the examples by default, when building the lib, and using my meson.build. Yes. This builds fine on x32, without error. only the specific example might fail.

@foxtrot
Copy link
Member

foxtrot commented Nov 13, 2022

Hey, sorry for the delay on this, I've been very busy recently. I had assumed wrongly that switching to %llu that, when targeting a 32-bit binary, the compiler would be happy with it. Considering this is just the example code, I will fix it to use the PRIu64 specifier at some point, but it's not currently urgent for me due to other life events right now, sorry.

@foxtrot foxtrot reopened this Nov 13, 2022
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

No branches or pull requests

2 participants