Re: [PATCH 1/5] Fix cpumask_scnprintf_len() misdesign

Re: [PATCH 1/5] Fix cpumask_scnprintf_len() misdesign

[Andrew Morton]

On Thu, 8 May 2008 02:14:50 +0400
Alexey Dobriyan <adobriyan@gmail.com> wrote:

> cpumask_scnprintf_len() takes "len" as an argument which is surprising,
> If one follows call chain down it isn't length but number of bits. But
> there is natural number of bits when talking about CPU masks -- NR_CPUS.
> So use that.

hm, maybe we should delete this function after all ;)

> Remove 1:1 wrapper macro while I'm at it.

There was a reason most of those 1:1 wrappers but yes, this one seems to be
rather cargo-cultish.

> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> ---
> 
>  include/linux/cpumask.h |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> --- a/include/linux/cpumask.h
> +++ b/include/linux/cpumask.h
> @@ -289,11 +289,9 @@ static inline int __cpumask_scnprintf(char *buf, int len,
>  	return bitmap_scnprintf(buf, len, srcp->bits, nbits);
>  }
>  
> -#define cpumask_scnprintf_len(len) \
> -			__cpumask_scnprintf_len((len))
> -static inline int __cpumask_scnprintf_len(int len)
> +static inline int cpumask_scnprintf_len(void)
>  {
> -	return bitmap_scnprintf_len(len);
> +	return bitmap_scnprintf_len(NR_CPUS);
>  }
>  
>  #define cpumask_parse_user(ubuf, ulen, dst) \

Does Paul have an opinion?

(Well, Paul always has an opinion.  But what is it?)

[PATCH 1/5] Fix cpumask_scnprintf_len() misdesign

[Alexey Dobriyan]

cpumask_scnprintf_len() takes "len" as an argument which is surprising,
If one follows call chain down it isn't length but number of bits. But
there is natural number of bits when talking about CPU masks -- NR_CPUS.
So use that.

Remove 1:1 wrapper macro while I'm at it.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 include/linux/cpumask.h |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -289,11 +289,9 @@ static inline int __cpumask_scnprintf(char *buf, int len,
 	return bitmap_scnprintf(buf, len, srcp->bits, nbits);
 }
 
-#define cpumask_scnprintf_len(len) \
-			__cpumask_scnprintf_len((len))
-static inline int __cpumask_scnprintf_len(int len)
+static inline int cpumask_scnprintf_len(void)
 {
-	return bitmap_scnprintf_len(len);
+	return bitmap_scnprintf_len(NR_CPUS);
 }
 
 #define cpumask_parse_user(ubuf, ulen, dst) \

Re: [PATCH 1/5] Fix cpumask_scnprintf_len() misdesign

[Paul Jackson]

Andrew asked:
> Does Paul have an opinion?

Of course ;).

I just sent a patch to remove bitmap_scnprintf_len and cpumask_scnprintf_len.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

Re: [PATCH 1/5] Fix cpumask_scnprintf_len() misdesign

[Bert Wesarg]

On Thu, May 8, 2008 at 12:14 AM, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> cpumask_scnprintf_len() takes "len" as an argument which is surprising,
>  If one follows call chain down it isn't length but number of bits. But
>  there is natural number of bits when talking about CPU masks -- NR_CPUS.
>  So use that.
Reported-here: http://thread.gmane.org/gmane.linux.kernel/671725

>
>  Remove 1:1 wrapper macro while I'm at it.
>
>  Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Acked-by: Bert Wesarg <bert.wesarg@googlemail.com>