| Message ID | 20240628223237.16222-1-rosenp@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | iw: fix formats under MIPS64/PPC | expand |
On Fri, 2024-06-28 at 15:32 -0700, Rosen Penev wrote: > __SANE_USERSPACE_TYPES__ needs to be defined to get consistent 64-bit > type defines and to fix -Wformat warnings. > How does this even work? Pretty much every file I checked in iw includes iw.h _last_ (or close to it), after netlink, nl80211.h etc., so ...? Doesn't really seem like it would have any effect? johannes
On Mon, Jul 1, 2024 at 3:01 AM Johannes Berg <johannes@sipsolutions.net> wrote: > > On Fri, 2024-06-28 at 15:32 -0700, Rosen Penev wrote: > > __SANE_USERSPACE_TYPES__ needs to be defined to get consistent 64-bit > > type defines and to fix -Wformat warnings. > > > > How does this even work? Pretty much every file I checked in iw includes > iw.h _last_ (or close to it), after netlink, nl80211.h etc., so ...? > Doesn't really seem like it would have any effect? I only tested compilation of iw itself, not of any project that uses it. > > johannes
On Mon, 2024-07-01 at 14:28 -0700, Rosen Penev wrote: > On Mon, Jul 1, 2024 at 3:01 AM Johannes Berg <johannes@sipsolutions.net> wrote: > > > > On Fri, 2024-06-28 at 15:32 -0700, Rosen Penev wrote: > > > __SANE_USERSPACE_TYPES__ needs to be defined to get consistent 64-bit > > > type defines and to fix -Wformat warnings. > > > > > > > How does this even work? Pretty much every file I checked in iw includes > > iw.h _last_ (or close to it), after netlink, nl80211.h etc., so ...? > > Doesn't really seem like it would have any effect? > I only tested compilation of iw itself, not of any project that uses it. Hm? It's iw itself only anyway? But where did you see warnings/errors, and why did they go away when this shouldn't have really done anything? johannes >
On Mon, Jul 1, 2024 at 2:58 PM Johannes Berg <johannes@sipsolutions.net> wrote: > > On Mon, 2024-07-01 at 14:28 -0700, Rosen Penev wrote: > > On Mon, Jul 1, 2024 at 3:01 AM Johannes Berg <johannes@sipsolutions.net> wrote: > > > > > > On Fri, 2024-06-28 at 15:32 -0700, Rosen Penev wrote: > > > > __SANE_USERSPACE_TYPES__ needs to be defined to get consistent 64-bit > > > > type defines and to fix -Wformat warnings. > > > > > > > > > > How does this even work? Pretty much every file I checked in iw includes > > > iw.h _last_ (or close to it), after netlink, nl80211.h etc., so ...? > > > Doesn't really seem like it would have any effect? > > I only tested compilation of iw itself, not of any project that uses it. > > Hm? It's iw itself only anyway? But where did you see warnings/errors, > and why did they go away when this shouldn't have really done anything? The whole codebase. They go away because if the define is found before any header inclusion, __u64 gets defined to unsigned long long. Note that the header after the define include linux headers. The impacted architectures can be found in the kernel with git grep SANE_USERSPACE arch > > johannes > >
On Mon, 2024-07-01 at 15:07 -0700, Rosen Penev wrote: > > They go away because if the define is found before any header > inclusion, __u64 gets defined to unsigned long long. It *isn't* found before any header inclusion though. For pretty much all of the C files, "iw.h" comes _last_ in the list of included headers. johannes
On Mon, Jul 1, 2024 at 3:10 PM Johannes Berg <johannes@sipsolutions.net> wrote: > > On Mon, 2024-07-01 at 15:07 -0700, Rosen Penev wrote: > > > > They go away because if the define is found before any header > > inclusion, __u64 gets defined to unsigned long long. > > It *isn't* found before any header inclusion though. > > For pretty much all of the C files, "iw.h" comes _last_ in the list of > included headers. Oh I see what you mean. No real idea. However without this patch, I get event.c: In function 'parse_nan_match': event.c:668:41: error: format '%llx' expects argument of type 'long long unsigned int', but argument 2 has type '__u64' {aka 'long unsigned int'} [-Werror=format=] 668 | "NAN(cookie=0x%llx): DiscoveryResult, peer_id=%d, local_id=%d, peer_mac=%s", | ~~~^ | | | long long unsigned int | %lx 669 | cookie, | ~~~~~~ | | | __u64 {aka long unsigned int} event.c:680:41: error: format '%llx' expects argument of type 'long long unsigned int', but argument 2 has type '__u64' {aka 'long unsigned int'} [-Werror=format=] 680 | "NAN(cookie=0x%llx): Replied, peer_id=%d, local_id=%d, peer_mac=%s", | ~~~^ | | | long long unsigned int | %lx 681 | cookie, | ~~~~~~ | | | __u64 {aka long unsigned int} event.c:688:41: error: format '%llx' expects argument of type 'long long unsigned int', but argument 2 has type '__u64' {aka 'long unsigned int'} [-Werror=format=] 688 | "NAN(cookie=0x%llx): FollowUpReceive, peer_id=%d, local_id=%d, peer_mac=%s", | ~~~^ | | | long long unsigned int | %lx 689 | cookie, | ~~~~~~ | | | __u64 {aka long unsigned int} And many others. I submitted a similar patch to fio and was advised to move the define into the Makefiles. Not too sure how to do that here. > > johannes
On Mon, 2024-07-01 at 15:16 -0700, Rosen Penev wrote: > On Mon, Jul 1, 2024 at 3:10 PM Johannes Berg <johannes@sipsolutions.net> wrote: > > > > On Mon, 2024-07-01 at 15:07 -0700, Rosen Penev wrote: > > > > > > They go away because if the define is found before any header > > > inclusion, __u64 gets defined to unsigned long long. > > > > It *isn't* found before any header inclusion though. > > > > For pretty much all of the C files, "iw.h" comes _last_ in the list of > > included headers. > Oh I see what you mean. No real idea. However without this patch, I get > > event.c: In function 'parse_nan_match': OK, well, event.c is one of those cases where indeed most things are included indirectly via iw.h, so it would actually work ... but most files don't do that. Maybe lucky that they don't use 64-bit types (yet)? > I submitted a similar patch to fio and was advised to move the define > into the Makefiles. Not too sure how to do that here. > Probably a good idea, just add CFLAGS += -D__SANE_USERSPACE_TYPES__ in the an appropriate place with the others. johannes
On Mon, Jul 1, 2024 at 3:19 PM Johannes Berg <johannes@sipsolutions.net> wrote: > > On Mon, 2024-07-01 at 15:16 -0700, Rosen Penev wrote: > > On Mon, Jul 1, 2024 at 3:10 PM Johannes Berg <johannes@sipsolutions.net> wrote: > > > > > > On Mon, 2024-07-01 at 15:07 -0700, Rosen Penev wrote: > > > > > > > > They go away because if the define is found before any header > > > > inclusion, __u64 gets defined to unsigned long long. > > > > > > It *isn't* found before any header inclusion though. > > > > > > For pretty much all of the C files, "iw.h" comes _last_ in the list of > > > included headers. > > Oh I see what you mean. No real idea. However without this patch, I get > > > > event.c: In function 'parse_nan_match': > > OK, well, event.c is one of those cases where indeed most things are > included indirectly via iw.h, so it would actually work ... but most > files don't do that. Maybe lucky that they don't use 64-bit types (yet)? > > > I submitted a similar patch to fio and was advised to move the define > > into the Makefiles. Not too sure how to do that here. > > > > Probably a good idea, just add > > CFLAGS += -D__SANE_USERSPACE_TYPES__ Right. My thinking was to match against the system being built. I guess it doesn't really hurt to just define everywhere. That's what this patch does anyway. > > in the an appropriate place with the others. > > johannes
diff --git a/iw.h b/iw.h index f416d6d..436723f 100644 --- a/iw.h +++ b/iw.h @@ -1,6 +1,8 @@ #ifndef __IW_H #define __IW_H +#define __SANE_USERSPACE_TYPES__ + #include <stdbool.h> #include <netlink/netlink.h> #include <netlink/genl/genl.h>
__SANE_USERSPACE_TYPES__ needs to be defined to get consistent 64-bit type defines and to fix -Wformat warnings. Signed-off-by: Rosen Penev <rosenp@gmail.com> --- iw.h | 2 ++ 1 file changed, 2 insertions(+)