[PATCH] wiphy: Relpace rawmemchr with memchr

[PATCH] wiphy: Relpace rawmemchr with memchr

[Robert Joslyn]

[-- Attachment #1: Type: text/plain, Size: 655 bytes --]

The rawmemchr function is a GNU extension, and not available in other
libc implementations, such as musl.
---
 src/wiphy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/wiphy.c b/src/wiphy.c
index 1da479db..141b3cb9 100644
--- a/src/wiphy.c
+++ b/src/wiphy.c
@@ -500,7 +500,7 @@ const uint8_t *wiphy_get_supported_rates(struct wiphy *wiphy, unsigned int band,
 
 	if (out_num)
 		*out_num =
-			(uint8_t *) rawmemchr(wiphy->supported_rates[band], 0) -
+			(uint8_t *) memchr(wiphy->supported_rates[band], 0, (size_t)-1) -
 			wiphy->supported_rates[band];
 
 	return wiphy->supported_rates[band];
-- 
2.24.1

Re: [PATCH] wiphy: Relpace rawmemchr with memchr

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1231 bytes --]

Hi Robert,

On 2/1/20 1:53 AM, Robert Joslyn wrote:
> The rawmemchr function is a GNU extension, and not available in other
> libc implementations, such as musl.
> ---
>   src/wiphy.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/wiphy.c b/src/wiphy.c
> index 1da479db..141b3cb9 100644
> --- a/src/wiphy.c
> +++ b/src/wiphy.c
> @@ -500,7 +500,7 @@ const uint8_t *wiphy_get_supported_rates(struct wiphy *wiphy, unsigned int band,
>   
>   	if (out_num)
>   		*out_num =
> -			(uint8_t *) rawmemchr(wiphy->supported_rates[band], 0) -
> +			(uint8_t *) memchr(wiphy->supported_rates[band], 0, (size_t)-1) -
>   			wiphy->supported_rates[band];
>   
>   	return wiphy->supported_rates[band];
> 

Unfortunately this causes gcc to complain:

src/wiphy.c: In function ‘wiphy_get_supported_rates’:
src/wiphy.c:503:16: error: ‘memchr’ specified size 18446744073709551615 
exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
   503 |    (uint8_t *) memchr(wiphy->supported_rates[band], 0, 
(size_t)-1) -
       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

can you try the attached patch instead?

Regards,
-Denis

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-build-Support-missing-rawmemchr.patch --]
[-- Type: text/x-patch, Size: 1464 bytes --]

From fcdddf2b726439e049992878f90da607414a1a47 Mon Sep 17 00:00:00 2001
From: Denis Kenzior <denkenz@gmail.com>
Date: Mon, 3 Feb 2020 11:54:28 -0600
Subject: [PATCH] build: Support missing rawmemchr

---
 configure.ac  |  1 +
 src/missing.h | 10 ++++++++++
 src/wiphy.c   |  1 +
 3 files changed, 12 insertions(+)

diff --git a/configure.ac b/configure.ac
index 5ae1401cae17..2d373a47ba68 100644
--- a/configure.ac
+++ b/configure.ac
@@ -128,6 +128,7 @@ AC_DEFINE_UNQUOTED(WIRED_STORAGEDIR, "${wired_storagedir}",
 			[Directory for Ethernet daemon storage files])
 
 AC_CHECK_FUNCS(explicit_bzero)
+AC_CHECK_FUNCS(rawmemchr)
 
 AC_CHECK_HEADERS(linux/types.h linux/if_alg.h)
 
diff --git a/src/missing.h b/src/missing.h
index 2bb210ae3c81..2cc80aee5d38 100644
--- a/src/missing.h
+++ b/src/missing.h
@@ -27,3 +27,13 @@ static inline void explicit_bzero(void *s, size_t n)
         __asm__ __volatile__ ("" : : "r"(s) : "memory");
 }
 #endif
+
+#ifndef HAVE_RAWMEMCHR
+static inline void *rawmemchr(const void *s, int c)
+{
+_Pragma("GCC diagnostic push")
+_Pragma("GCC diagnostic ignored \"-Wstringop-overflow=\"")
+	return memchr(s, c, (size_t) -1);
+_Pragma("GCC diagnostic pop")
+}
+#endif
diff --git a/src/wiphy.c b/src/wiphy.c
index 1da479db2dab..511bb27f52b8 100644
--- a/src/wiphy.c
+++ b/src/wiphy.c
@@ -37,6 +37,7 @@
 
 #include "linux/nl80211.h"
 
+#include "src/missing.h"
 #include "src/iwd.h"
 #include "src/module.h"
 #include "src/ie.h"
-- 
2.21.0

Re: [PATCH] wiphy: Relpace rawmemchr with memchr

[Robert Joslyn]

[-- Attachment #1: Type: text/plain, Size: 2026 bytes --]

On Mon, 2020-02-03 at 11:55 -0600, Denis Kenzior wrote:
> Hi Robert,
> 
> On 2/1/20 1:53 AM, Robert Joslyn wrote:
> > The rawmemchr function is a GNU extension, and not available in other
> > libc implementations, such as musl.
> > ---
> >   src/wiphy.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/wiphy.c b/src/wiphy.c
> > index 1da479db..141b3cb9 100644
> > --- a/src/wiphy.c
> > +++ b/src/wiphy.c
> > @@ -500,7 +500,7 @@ const uint8_t *wiphy_get_supported_rates(struct
> > wiphy *wiphy, unsigned int band,
> >   
> >   	if (out_num)
> >   		*out_num =
> > -			(uint8_t *) rawmemchr(wiphy-
> > >supported_rates[band], 0) -
> > +			(uint8_t *) memchr(wiphy->supported_rates[band],
> > 0, (size_t)-1) -
> >   			wiphy->supported_rates[band];
> >   
> >   	return wiphy->supported_rates[band];
> > 
> 
> Unfortunately this causes gcc to complain:
> 
> src/wiphy.c: In function ‘wiphy_get_supported_rates’:
> src/wiphy.c:503:16: error: ‘memchr’ specified size 18446744073709551615 
> exceeds maximum object size 9223372036854775807 [-Werror=stringop-
> overflow=]
>    503 |    (uint8_t *) memchr(wiphy->supported_rates[band], 0, 
> (size_t)-1) -
>        |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~
> 
> can you try the attached patch instead?
> 
> Regards,
> -Denis

Hi Denis,

Sorry about that. I'm doing my builds with Yocto/OpenEmbedded (master
branch) and was able to build with both glibc and musl with my original
patch, but compile flags or gcc version are probably different.

Your patch works fine for me and looks like a better approach.

This got me curious, so I looked at what glibc does: 
http://sourceware.org/git/?p=glibc.git;a=blob;f=string/rawmemchr.c;h=b62d285d7efa882b3928fae03318273d588f15bd;hb=HEAD

I expect the use of strlen when looking for null is slightly faster, but
probably doesn't really matter here.

Thanks for taking a look and working on a fix!

Robert

Re: [PATCH] wiphy: Relpace rawmemchr with memchr

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 674 bytes --]

Hi Robert,

> Your patch works fine for me and looks like a better approach.

Great!  Thanks for testing.

> 
> This got me curious, so I looked at what glibc does:
> http://sourceware.org/git/?p=glibc.git;a=blob;f=string/rawmemchr.c;h=b62d285d7efa882b3928fae03318273d588f15bd;hb=HEAD
> 
> I expect the use of strlen when looking for null is slightly faster, but
> probably doesn't really matter here.

Yeah, strlen might be a bit faster indeed.  But I was feeling lazy and 
this isn't used for anything performance critical. :)

> 
> Thanks for taking a look and working on a fix!
> 

No worries.  I pushed out this fix upstream now.

Regards,
-Denis