* [PATCH ethtool 0/3] Build fixes for older kernel headers and musl
@ 2023-01-13 23:31 Florian Fainelli
2023-01-13 23:31 ` [PATCH ethtool 1/3] misc: Fix build with kernel headers < v4.11 Florian Fainelli
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Florian Fainelli @ 2023-01-13 23:31 UTC (permalink / raw)
To: netdev; +Cc: Florian Fainelli, Michal Kubecek, Markus Mayer, Andrew Lunn
Hi Michal,
These 3 patches fix build issues encountered in the 6.1 release with
either older kernel headers (pre v4.11) or with musl-libc.
In case you want to add a prebuilt toolchain with your release procedure
you can use those binaries:
https://github.com/Broadcom/stbgcc-8.3/releases/tag/stbgcc-8.3-0.4
Florian Fainelli (3):
misc: Fix build with kernel headers < v4.11
netlink: Fix maybe uninitialized 'meters' variable
marvell.c: Fix build with musl-libc
internal.h | 1 +
marvell.c | 2 +-
netlink/parser.c | 2 +-
3 files changed, 3 insertions(+), 2 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH ethtool 1/3] misc: Fix build with kernel headers < v4.11
2023-01-13 23:31 [PATCH ethtool 0/3] Build fixes for older kernel headers and musl Florian Fainelli
@ 2023-01-13 23:31 ` Florian Fainelli
2023-01-13 23:57 ` Michal Kubecek
2023-01-13 23:31 ` [PATCH ethtool 2/3] netlink: Fix maybe uninitialized 'meters' variable Florian Fainelli
2023-01-13 23:31 ` [PATCH ethtool 3/3] marvell.c: Fix build with musl-libc Florian Fainelli
2 siblings, 1 reply; 11+ messages in thread
From: Florian Fainelli @ 2023-01-13 23:31 UTC (permalink / raw)
To: netdev; +Cc: Florian Fainelli, Markus Mayer, Michal Kubecek, Andrew Lunn
Not all toolchain kernel headers may contain upstream commit
2618be7dccf8739b89e1906b64bd8d551af351e6 ("uapi: fix linux/if.h
userspace compilation errors") which is included in v4.11 and onwards.
Err on the side of caution by including sys/socket.h ahead of including
linux/if.h.
Fixes: 1fa60003a8b8 ("misc: header includes cleanup")
Reported-by: Markus Mayer <mmayer@broadcom.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
internal.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/internal.h b/internal.h
index b80f77afa4c0..f7aaaf5229f4 100644
--- a/internal.h
+++ b/internal.h
@@ -21,6 +21,7 @@
#include <unistd.h>
#include <endian.h>
#include <sys/ioctl.h>
+#include <sys/socket.h>
#include <linux/if.h>
#include "json_writer.h"
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH ethtool 2/3] netlink: Fix maybe uninitialized 'meters' variable
2023-01-13 23:31 [PATCH ethtool 0/3] Build fixes for older kernel headers and musl Florian Fainelli
2023-01-13 23:31 ` [PATCH ethtool 1/3] misc: Fix build with kernel headers < v4.11 Florian Fainelli
@ 2023-01-13 23:31 ` Florian Fainelli
2023-01-14 0:09 ` Michal Kubecek
2023-01-13 23:31 ` [PATCH ethtool 3/3] marvell.c: Fix build with musl-libc Florian Fainelli
2 siblings, 1 reply; 11+ messages in thread
From: Florian Fainelli @ 2023-01-13 23:31 UTC (permalink / raw)
To: netdev; +Cc: Florian Fainelli, Michal Kubecek, Markus Mayer, Andrew Lunn
GCC12 warns that 'meters' may be uninitialized, initialize it
accordingly.
Fixes: 9561db9b76f4 ("Add cable test TDR support")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
netlink/parser.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/netlink/parser.c b/netlink/parser.c
index f982f229a040..6f863610a490 100644
--- a/netlink/parser.c
+++ b/netlink/parser.c
@@ -237,7 +237,7 @@ int nl_parse_direct_m2cm(struct nl_context *nlctx, uint16_t type,
struct nl_msg_buff *msgbuff, void *dest)
{
const char *arg = *nlctx->argp;
- float meters;
+ float meters = 0.0;
uint32_t cm;
int ret;
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH ethtool 3/3] marvell.c: Fix build with musl-libc
2023-01-13 23:31 [PATCH ethtool 0/3] Build fixes for older kernel headers and musl Florian Fainelli
2023-01-13 23:31 ` [PATCH ethtool 1/3] misc: Fix build with kernel headers < v4.11 Florian Fainelli
2023-01-13 23:31 ` [PATCH ethtool 2/3] netlink: Fix maybe uninitialized 'meters' variable Florian Fainelli
@ 2023-01-13 23:31 ` Florian Fainelli
2023-01-14 0:13 ` Michal Kubecek
2 siblings, 1 reply; 11+ messages in thread
From: Florian Fainelli @ 2023-01-13 23:31 UTC (permalink / raw)
To: netdev; +Cc: Florian Fainelli, Michal Kubecek, Markus Mayer, Andrew Lunn
After commit 1fa60003a8b8 ("misc: header includes cleanup") we stopped
including net/if.h which resolved the proper defines to pull in
sys/types.h and provide a definition for u_int32_t. With musl-libc we
need to define _GNU_SOURCE to ensure that sys/types.h does provide a
definition for u_int32_t.
Fixes: 1fa60003a8b8 ("misc: header includes cleanup")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
marvell.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/marvell.c b/marvell.c
index d3d570e4d4ad..be2fc36b8fc5 100644
--- a/marvell.c
+++ b/marvell.c
@@ -6,7 +6,7 @@
*/
#include <stdio.h>
-
+#define _GNU_SOURCE
#include "internal.h"
static void dump_addr(int n, const u8 *a)
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH ethtool 1/3] misc: Fix build with kernel headers < v4.11
2023-01-13 23:31 ` [PATCH ethtool 1/3] misc: Fix build with kernel headers < v4.11 Florian Fainelli
@ 2023-01-13 23:57 ` Michal Kubecek
2023-01-14 0:11 ` Florian Fainelli
0 siblings, 1 reply; 11+ messages in thread
From: Michal Kubecek @ 2023-01-13 23:57 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, Markus Mayer, Andrew Lunn
[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]
On Fri, Jan 13, 2023 at 03:31:46PM -0800, Florian Fainelli wrote:
> Not all toolchain kernel headers may contain upstream commit
> 2618be7dccf8739b89e1906b64bd8d551af351e6 ("uapi: fix linux/if.h
> userspace compilation errors") which is included in v4.11 and onwards.
> Err on the side of caution by including sys/socket.h ahead of including
> linux/if.h.
>
> Fixes: 1fa60003a8b8 ("misc: header includes cleanup")
> Reported-by: Markus Mayer <mmayer@broadcom.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> internal.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/internal.h b/internal.h
> index b80f77afa4c0..f7aaaf5229f4 100644
> --- a/internal.h
> +++ b/internal.h
> @@ -21,6 +21,7 @@
> #include <unistd.h>
> #include <endian.h>
> #include <sys/ioctl.h>
> +#include <sys/socket.h>
> #include <linux/if.h>
>
> #include "json_writer.h"
No objection but I wonder if it wouldn't make sense to add linux/if.h to
the header copies in uapi/ instead as then we could also drop the
fallback definition of ALTIFNAMSIZ and perhaps more similar hacks.
Michal
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH ethtool 2/3] netlink: Fix maybe uninitialized 'meters' variable
2023-01-13 23:31 ` [PATCH ethtool 2/3] netlink: Fix maybe uninitialized 'meters' variable Florian Fainelli
@ 2023-01-14 0:09 ` Michal Kubecek
2023-01-14 0:12 ` Florian Fainelli
0 siblings, 1 reply; 11+ messages in thread
From: Michal Kubecek @ 2023-01-14 0:09 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, Markus Mayer, Andrew Lunn
[-- Attachment #1: Type: text/plain, Size: 1191 bytes --]
On Fri, Jan 13, 2023 at 03:31:47PM -0800, Florian Fainelli wrote:
> GCC12 warns that 'meters' may be uninitialized, initialize it
> accordingly.
>
> Fixes: 9561db9b76f4 ("Add cable test TDR support")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> netlink/parser.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/netlink/parser.c b/netlink/parser.c
> index f982f229a040..6f863610a490 100644
> --- a/netlink/parser.c
> +++ b/netlink/parser.c
> @@ -237,7 +237,7 @@ int nl_parse_direct_m2cm(struct nl_context *nlctx, uint16_t type,
> struct nl_msg_buff *msgbuff, void *dest)
> {
> const char *arg = *nlctx->argp;
> - float meters;
> + float meters = 0.0;
> uint32_t cm;
> int ret;
>
No problem here either but it's quite surprising as I check build with
gcc versions 7 and 11-13 (10-12 until recently) for each new commit and
I never saw this warning. As the warning is actually incorrect (either
parse_float() returns an error and we bail out or it assigns a value to
meters), it may be a gcc issue that was fixed in a later version. But
initializing the variable does no harm so let's do it.
Michal
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH ethtool 1/3] misc: Fix build with kernel headers < v4.11
2023-01-13 23:57 ` Michal Kubecek
@ 2023-01-14 0:11 ` Florian Fainelli
2023-01-14 2:07 ` Florian Fainelli
0 siblings, 1 reply; 11+ messages in thread
From: Florian Fainelli @ 2023-01-14 0:11 UTC (permalink / raw)
To: Michal Kubecek; +Cc: netdev, Markus Mayer, Andrew Lunn
On 1/13/2023 3:57 PM, Michal Kubecek wrote:
> On Fri, Jan 13, 2023 at 03:31:46PM -0800, Florian Fainelli wrote:
>> Not all toolchain kernel headers may contain upstream commit
>> 2618be7dccf8739b89e1906b64bd8d551af351e6 ("uapi: fix linux/if.h
>> userspace compilation errors") which is included in v4.11 and onwards.
>> Err on the side of caution by including sys/socket.h ahead of including
>> linux/if.h.
>>
>> Fixes: 1fa60003a8b8 ("misc: header includes cleanup")
>> Reported-by: Markus Mayer <mmayer@broadcom.com>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> internal.h | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/internal.h b/internal.h
>> index b80f77afa4c0..f7aaaf5229f4 100644
>> --- a/internal.h
>> +++ b/internal.h
>> @@ -21,6 +21,7 @@
>> #include <unistd.h>
>> #include <endian.h>
>> #include <sys/ioctl.h>
>> +#include <sys/socket.h>
>> #include <linux/if.h>
>>
>> #include "json_writer.h"
>
> No objection but I wonder if it wouldn't make sense to add linux/if.h to
> the header copies in uapi/ instead as then we could also drop the
> fallback definition of ALTIFNAMSIZ and perhaps more similar hacks.
Humm, I wondered about that but it seems like opening a possible can of
worms as the history of include/uapi/linux/if.h changed across multiple
versions. As long as it does compile, why not :)
--
Florian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH ethtool 2/3] netlink: Fix maybe uninitialized 'meters' variable
2023-01-14 0:09 ` Michal Kubecek
@ 2023-01-14 0:12 ` Florian Fainelli
0 siblings, 0 replies; 11+ messages in thread
From: Florian Fainelli @ 2023-01-14 0:12 UTC (permalink / raw)
To: Michal Kubecek; +Cc: netdev, Markus Mayer, Andrew Lunn
On 1/13/2023 4:09 PM, Michal Kubecek wrote:
> On Fri, Jan 13, 2023 at 03:31:47PM -0800, Florian Fainelli wrote:
>> GCC12 warns that 'meters' may be uninitialized, initialize it
>> accordingly.
>>
>> Fixes: 9561db9b76f4 ("Add cable test TDR support")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> netlink/parser.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/netlink/parser.c b/netlink/parser.c
>> index f982f229a040..6f863610a490 100644
>> --- a/netlink/parser.c
>> +++ b/netlink/parser.c
>> @@ -237,7 +237,7 @@ int nl_parse_direct_m2cm(struct nl_context *nlctx, uint16_t type,
>> struct nl_msg_buff *msgbuff, void *dest)
>> {
>> const char *arg = *nlctx->argp;
>> - float meters;
>> + float meters = 0.0;
>> uint32_t cm;
>> int ret;
>>
>
> No problem here either but it's quite surprising as I check build with
> gcc versions 7 and 11-13 (10-12 until recently) for each new commit and
> I never saw this warning. As the warning is actually incorrect (either
> parse_float() returns an error and we bail out or it assigns a value to
> meters), it may be a gcc issue that was fixed in a later version. But
> initializing the variable does no harm so let's do it.
Yes that might be the case, this was seen with GCC 11.3 as well. I
definitively did use a GCC12 pre-release at some point, too.
--
Florian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH ethtool 3/3] marvell.c: Fix build with musl-libc
2023-01-13 23:31 ` [PATCH ethtool 3/3] marvell.c: Fix build with musl-libc Florian Fainelli
@ 2023-01-14 0:13 ` Michal Kubecek
2023-01-14 2:07 ` Florian Fainelli
0 siblings, 1 reply; 11+ messages in thread
From: Michal Kubecek @ 2023-01-14 0:13 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, Markus Mayer, Andrew Lunn
[-- Attachment #1: Type: text/plain, Size: 1083 bytes --]
On Fri, Jan 13, 2023 at 03:31:48PM -0800, Florian Fainelli wrote:
> After commit 1fa60003a8b8 ("misc: header includes cleanup") we stopped
> including net/if.h which resolved the proper defines to pull in
> sys/types.h and provide a definition for u_int32_t. With musl-libc we
> need to define _GNU_SOURCE to ensure that sys/types.h does provide a
> definition for u_int32_t.
>
> Fixes: 1fa60003a8b8 ("misc: header includes cleanup")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> marvell.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/marvell.c b/marvell.c
> index d3d570e4d4ad..be2fc36b8fc5 100644
> --- a/marvell.c
> +++ b/marvell.c
> @@ -6,7 +6,7 @@
> */
>
> #include <stdio.h>
> -
> +#define _GNU_SOURCE
> #include "internal.h"
>
> static void dump_addr(int n, const u8 *a)
I would prefer replacing u_intXX_t types with standard uintXX_t and
including <stdint.h>. That would be consistent with the rest of the
code which uses ISO uintXX_t types or (older code) kernel uXX types.
Michal
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH ethtool 3/3] marvell.c: Fix build with musl-libc
2023-01-14 0:13 ` Michal Kubecek
@ 2023-01-14 2:07 ` Florian Fainelli
0 siblings, 0 replies; 11+ messages in thread
From: Florian Fainelli @ 2023-01-14 2:07 UTC (permalink / raw)
To: Michal Kubecek; +Cc: netdev, Markus Mayer, Andrew Lunn
On 1/13/2023 4:13 PM, Michal Kubecek wrote:
> On Fri, Jan 13, 2023 at 03:31:48PM -0800, Florian Fainelli wrote:
>> After commit 1fa60003a8b8 ("misc: header includes cleanup") we stopped
>> including net/if.h which resolved the proper defines to pull in
>> sys/types.h and provide a definition for u_int32_t. With musl-libc we
>> need to define _GNU_SOURCE to ensure that sys/types.h does provide a
>> definition for u_int32_t.
>>
>> Fixes: 1fa60003a8b8 ("misc: header includes cleanup")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> marvell.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/marvell.c b/marvell.c
>> index d3d570e4d4ad..be2fc36b8fc5 100644
>> --- a/marvell.c
>> +++ b/marvell.c
>> @@ -6,7 +6,7 @@
>> */
>>
>> #include <stdio.h>
>> -
>> +#define _GNU_SOURCE
>> #include "internal.h"
>>
>> static void dump_addr(int n, const u8 *a)
>
> I would prefer replacing u_intXX_t types with standard uintXX_t and
> including <stdint.h>. That would be consistent with the rest of the
> code which uses ISO uintXX_t types or (older code) kernel uXX types.
Sounds good, I will do that in v2.
--
Florian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH ethtool 1/3] misc: Fix build with kernel headers < v4.11
2023-01-14 0:11 ` Florian Fainelli
@ 2023-01-14 2:07 ` Florian Fainelli
0 siblings, 0 replies; 11+ messages in thread
From: Florian Fainelli @ 2023-01-14 2:07 UTC (permalink / raw)
To: Michal Kubecek; +Cc: netdev, Markus Mayer, Andrew Lunn
On 1/13/2023 4:11 PM, Florian Fainelli wrote:
>
>
> On 1/13/2023 3:57 PM, Michal Kubecek wrote:
>> On Fri, Jan 13, 2023 at 03:31:46PM -0800, Florian Fainelli wrote:
>>> Not all toolchain kernel headers may contain upstream commit
>>> 2618be7dccf8739b89e1906b64bd8d551af351e6 ("uapi: fix linux/if.h
>>> userspace compilation errors") which is included in v4.11 and onwards.
>>> Err on the side of caution by including sys/socket.h ahead of including
>>> linux/if.h.
>>>
>>> Fixes: 1fa60003a8b8 ("misc: header includes cleanup")
>>> Reported-by: Markus Mayer <mmayer@broadcom.com>
>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>> ---
>>> internal.h | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/internal.h b/internal.h
>>> index b80f77afa4c0..f7aaaf5229f4 100644
>>> --- a/internal.h
>>> +++ b/internal.h
>>> @@ -21,6 +21,7 @@
>>> #include <unistd.h>
>>> #include <endian.h>
>>> #include <sys/ioctl.h>
>>> +#include <sys/socket.h>
>>> #include <linux/if.h>
>>> #include "json_writer.h"
>>
>> No objection but I wonder if it wouldn't make sense to add linux/if.h to
>> the header copies in uapi/ instead as then we could also drop the
>> fallback definition of ALTIFNAMSIZ and perhaps more similar hacks.
>
> Humm, I wondered about that but it seems like opening a possible can of
> worms as the history of include/uapi/linux/if.h changed across multiple
> versions. As long as it does compile, why not :)
Just to be clear, I will give that one a shot and let you know how it
goes, if it works fine, then it will be in v2.
--
Florian
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-01-14 2:07 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-13 23:31 [PATCH ethtool 0/3] Build fixes for older kernel headers and musl Florian Fainelli
2023-01-13 23:31 ` [PATCH ethtool 1/3] misc: Fix build with kernel headers < v4.11 Florian Fainelli
2023-01-13 23:57 ` Michal Kubecek
2023-01-14 0:11 ` Florian Fainelli
2023-01-14 2:07 ` Florian Fainelli
2023-01-13 23:31 ` [PATCH ethtool 2/3] netlink: Fix maybe uninitialized 'meters' variable Florian Fainelli
2023-01-14 0:09 ` Michal Kubecek
2023-01-14 0:12 ` Florian Fainelli
2023-01-13 23:31 ` [PATCH ethtool 3/3] marvell.c: Fix build with musl-libc Florian Fainelli
2023-01-14 0:13 ` Michal Kubecek
2023-01-14 2:07 ` Florian Fainelli
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.