[PATCH] iw: fix formats under MIPS64/PPC

[PATCH] iw: fix formats under MIPS64/PPC

[Rosen Penev]

__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>
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile b/Makefile
index 17be33f..2652fac 100644
--- a/Makefile
+++ b/Makefile
@@ -17,6 +17,7 @@ CFLAGS ?= -O2 -g
 CFLAGS += -Wall -Wextra -Wundef -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common
 CFLAGS += -Werror-implicit-function-declaration -Wsign-compare -Wno-unused-parameter
 CFLAGS += -Wdeclaration-after-statement
+CFLAGS += -D__SANE_USERSPACE_TYPES__
 CFLAGS += $(CFLAGS_EVAL)
 CFLAGS += $(EXTRA_CFLAGS)
 
-- 
2.45.2

[PATCH] iw: fix formats under MIPS64/PPC

[Rosen Penev]

__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(+)

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>
-- 
2.45.2

Re: [PATCH] iw: fix formats under MIPS64/PPC

[Johannes Berg]

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

Re: [PATCH] iw: fix formats under MIPS64/PPC

[Rosen Penev]

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

Re: [PATCH] iw: fix formats under MIPS64/PPC

[Johannes Berg]

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
> 

Re: [PATCH] iw: fix formats under MIPS64/PPC

[Rosen Penev]

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
> >

Re: [PATCH] iw: fix formats under MIPS64/PPC

[Johannes Berg]

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

Re: [PATCH] iw: fix formats under MIPS64/PPC

[Rosen Penev]

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

Re: [PATCH] iw: fix formats under MIPS64/PPC

[Johannes Berg]

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

Re: [PATCH] iw: fix formats under MIPS64/PPC

[Rosen Penev]

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