[B.A.T.M.A.N.] [PATCH] batman-adv: add compat code for kstrtou32

[B.A.T.M.A.N.] [PATCH] batman-adv: add compat code for kstrtou32

[Antonio Quartulli]

kstrtou32() does not exist before 2.6.39. Convert it to
strict_strtoul() in that case.

Used-by: e29249b4483b91d7a590a16e9b5ffe9412ce7fde
("batman-adv: add isolation_mark sysfs attribute")

Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
---
 compat.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/compat.h b/compat.h
index 01984b8..334dc5c 100644
--- a/compat.h
+++ b/compat.h
@@ -152,6 +152,8 @@ static inline int batadv_param_set_copystring(const char *val,
 
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 39)
 
+/* cast last argument of strict_strtoul() because we have a uint32_t */
+#define kstrtou32(cp, base, v) strict_strtoul(cp, base, (unsigned long *)v)
 #define kstrtoul strict_strtoul
 #define kstrtol  strict_strtol
 
-- 
1.8.4.3

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: add compat code for kstrtou32

[Marek Lindner]

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

On Tuesday 19 November 2013 08:26:07 Antonio Quartulli wrote:
> kstrtou32() does not exist before 2.6.39. Convert it to
> strict_strtoul() in that case.
> 
> Used-by: e29249b4483b91d7a590a16e9b5ffe9412ce7fde
> ("batman-adv: add isolation_mark sysfs attribute")
> 
> Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
> ---
>  compat.h | 2 ++
>  1 file changed, 2 insertions(+)

Applied in revision 28855a6.

Thanks,
Marek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: add compat code for kstrtou32

[Matthias Schiffer]

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

On 11/19/2013 08:26 AM, Antonio Quartulli wrote:
> kstrtou32() does not exist before 2.6.39. Convert it to
> strict_strtoul() in that case.
> 
> Used-by: e29249b4483b91d7a590a16e9b5ffe9412ce7fde
> ("batman-adv: add isolation_mark sysfs attribute")
> 
> Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
> ---
>  compat.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/compat.h b/compat.h
> index 01984b8..334dc5c 100644
> --- a/compat.h
> +++ b/compat.h
> @@ -152,6 +152,8 @@ static inline int batadv_param_set_copystring(const char *val,
>  
>  #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 39)
>  
> +/* cast last argument of strict_strtoul() because we have a uint32_t */
> +#define kstrtou32(cp, base, v) strict_strtoul(cp, base, (unsigned long *)v)
>  #define kstrtoul strict_strtoul
>  #define kstrtol  strict_strtol
>  
> 

You are casting a u32 * to unsigned long *? Won't this break horribly
when sizeof(u32) != sizeof(unsigned long)?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: add compat code for kstrtou32

[Antonio Quartulli]

[-- Attachment #1.1: Type: text/plain, Size: 432 bytes --]

On Thu, Nov 21, 2013 at 04:45:14AM +0100, Matthias Schiffer wrote:
> You are casting a u32 * to unsigned long *? Won't this break horribly
> when sizeof(u32) != sizeof(unsigned long)?
> 

It will break when sizeof(unsigned long) > sizeof(uint32). For example on
x86_64. Hopefully nobody would use such ancient kernel on his machine. But you
are right.

What do you think of the attached patch?


-- 
Antonio Quartulli

[-- Attachment #1.2: patch --]
[-- Type: text/plain, Size: 608 bytes --]

diff --git a/compat.h b/compat.h
index a050bb8..fe64820 100644
--- a/compat.h
+++ b/compat.h
@@ -153,7 +153,14 @@ static inline int batadv_param_set_copystring(const char *val,
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 39)
 
 /* cast last argument of strict_strtoul() because we have a uint32_t */
-#define kstrtou32(cp, base, v) strict_strtoul(cp, base, (unsigned long *)v)
+#define kstrtou32(cp, base, v) \
+({\
+	unsigned long _v;\
+	int _r;\
+	_r = strict_strtoul(cp, base, _v);\
+	v = (uint32_t)_v;\
+	_r;\
+})
 #define kstrtoul strict_strtoul
 #define kstrtol  strict_strtol
 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: add compat code for kstrtou32

[Matthias Schiffer]

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

On 11/21/2013 08:21 AM, Antonio Quartulli wrote:
> On Thu, Nov 21, 2013 at 04:45:14AM +0100, Matthias Schiffer wrote:
>> You are casting a u32 * to unsigned long *? Won't this break horribly
>> when sizeof(u32) != sizeof(unsigned long)?
>>
> 
> It will break when sizeof(unsigned long) > sizeof(uint32). For example on
> x86_64. Hopefully nobody would use such ancient kernel on his machine. But you
> are right.
> 
> What do you think of the attached patch?
> 

That's better, but if you care about overflow, you should add something like

if (_v != (unsigned long)v)
	_r = -ERANGE;

Also, the comment in the line above doesn't make sense anymore...


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: add compat code for kstrtou32

[Antonio Quartulli]

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

On Thu, Nov 21, 2013 at 02:57:50PM +0100, Matthias Schiffer wrote:
> On 11/21/2013 08:21 AM, Antonio Quartulli wrote:
> > On Thu, Nov 21, 2013 at 04:45:14AM +0100, Matthias Schiffer wrote:
> >> You are casting a u32 * to unsigned long *? Won't this break horribly
> >> when sizeof(u32) != sizeof(unsigned long)?
> >>
> > 
> > It will break when sizeof(unsigned long) > sizeof(uint32). For example on
> > x86_64. Hopefully nobody would use such ancient kernel on his machine. But you
> > are right.
> > 
> > What do you think of the attached patch?
> > 
> 
> That's better, but if you care about overflow, you should add something like
> 
> if (_v != (unsigned long)v)
> 	_r = -ERANGE;

yeah, thanks! I'll add this!

> 
> Also, the comment in the line above doesn't make sense anymore...

yeah, I removed that already.


-- 
Antonio Quartulli

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: add compat code for kstrtou32

[Matthias Schiffer]

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

On 11/21/2013 03:20 PM, Antonio Quartulli wrote:
> On Thu, Nov 21, 2013 at 02:57:50PM +0100, Matthias Schiffer wrote:
>> On 11/21/2013 08:21 AM, Antonio Quartulli wrote:
>>> On Thu, Nov 21, 2013 at 04:45:14AM +0100, Matthias Schiffer wrote:
>>>> You are casting a u32 * to unsigned long *? Won't this break horribly
>>>> when sizeof(u32) != sizeof(unsigned long)?
>>>>
>>>
>>> It will break when sizeof(unsigned long) > sizeof(uint32). For example on
>>> x86_64. Hopefully nobody would use such ancient kernel on his machine. But you
>>> are right.
>>>
>>> What do you think of the attached patch?
>>>
>>
>> That's better, but if you care about overflow, you should add something like
>>
>> if (_v != (unsigned long)v)
>> 	_r = -ERANGE;
> 
> yeah, thanks! I'll add this!
> 
>>
>> Also, the comment in the line above doesn't make sense anymore...
> 
> yeah, I removed that already.
> 
> 

Ah, and I overlooked that you have to dereference v and take the address
of _v; adding parantheses around the parameters might also be a good
idea as it is a macro.

So I'd recommend something like this (not even compile tested...):

({\
	unsigned long _v;\
	int _r;\
	_r = strict_strtoul((cp), (base), &_v);\
	if (_v == (unsigned long)(u32)_v)\
		*(v) = (u32)_v;\
	else\
		_r = -ERANGE;\
	_r;\
})



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: add compat code for kstrtou32

[Antonio Quartulli]

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

On Thu, Nov 21, 2013 at 03:31:23PM +0100, Matthias Schiffer wrote:
> On 11/21/2013 03:20 PM, Antonio Quartulli wrote:
> > On Thu, Nov 21, 2013 at 02:57:50PM +0100, Matthias Schiffer wrote:
> >> On 11/21/2013 08:21 AM, Antonio Quartulli wrote:
> >>> On Thu, Nov 21, 2013 at 04:45:14AM +0100, Matthias Schiffer wrote:
> >>>> You are casting a u32 * to unsigned long *? Won't this break horribly
> >>>> when sizeof(u32) != sizeof(unsigned long)?
> >>>>
> >>>
> >>> It will break when sizeof(unsigned long) > sizeof(uint32). For example on
> >>> x86_64. Hopefully nobody would use such ancient kernel on his machine. But you
> >>> are right.
> >>>
> >>> What do you think of the attached patch?
> >>>
> >>
> >> That's better, but if you care about overflow, you should add something like
> >>
> >> if (_v != (unsigned long)v)
> >> 	_r = -ERANGE;
> > 
> > yeah, thanks! I'll add this!
> > 
> >>
> >> Also, the comment in the line above doesn't make sense anymore...
> > 
> > yeah, I removed that already.
> > 
> > 
> 
> Ah, and I overlooked that you have to dereference v and take the address
> of _v;

yeah yeah, this snippet has not been compiled at all.

> adding parantheses around the parameters might also be a good
> idea as it is a macro.
> 

parenthesis in strict_strtoul() invocation are not useful, but yeah, they are
good around the rest.


-- 
Antonio Quartulli

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]