[2/5,RESEND] x86: avoid unneeded __div64_32 function definition

  From: Arnd Bergmann <arnd@arndb.de>

The __div64_32() function is provided for 32-bit architectures that
don't have a custom do_div() implementation. x86_32 has one, and
does not use the header file that declares the function prototype,
so the definition causes a W=1 warning:

lib/math/div64.c:31:32: error: no previous prototype for '__div64_32' [-Werror=missing-prototypes]

Define an empty macro to prevent the function definition from getting
built, which avoids the warning and saves a little .text space.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/x86/include/asm/div64.h | 2 ++
 1 file changed, 2 insertions(+)
  

On Tue, Jul 25, 2023 at 03:48:34PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The __div64_32() function is provided for 32-bit architectures that
> don't have a custom do_div() implementation. x86_32 has one, and
> does not use the header file that declares the function prototype,
> so the definition causes a W=1 warning:
> 
> lib/math/div64.c:31:32: error: no previous prototype for '__div64_32' [-Werror=missing-prototypes]
> 
> Define an empty macro to prevent the function definition from getting
> built, which avoids the warning and saves a little .text space.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/x86/include/asm/div64.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/x86/include/asm/div64.h b/arch/x86/include/asm/div64.h
> index b8f1dc0761e4b..9826d5fc12e34 100644
> --- a/arch/x86/include/asm/div64.h
> +++ b/arch/x86/include/asm/div64.h
> @@ -71,6 +71,8 @@ static inline u64 mul_u32_u32(u32 a, u32 b)
>  }
>  #define mul_u32_u32 mul_u32_u32
>  
> +#define __div64_32 /* not needed */

This comment, *after* having read the commit message makes sense.

When you look at it alone, after having opened the file, makes me
scratch my head and wonder what is that thing supposed to mean. Please
extend it.

And put the comment ontop, not sideways.

Thx.
  

On Tue, Aug 1, 2023, at 19:03, Borislav Petkov wrote:
> On Tue, Jul 25, 2023 at 03:48:34PM +0200, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>> 
>> The __div64_32() function is provided for 32-bit architectures that
>> don't have a custom do_div() implementation. x86_32 has one, and
>> does not use the header file that declares the function prototype,
>> so the definition causes a W=1 warning:
>> 
>> lib/math/div64.c:31:32: error: no previous prototype for '__div64_32' [-Werror=missing-prototypes]
>> 
>> Define an empty macro to prevent the function definition from getting
>> built, which avoids the warning and saves a little .text space.
>> 
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>>  arch/x86/include/asm/div64.h | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/arch/x86/include/asm/div64.h b/arch/x86/include/asm/div64.h
>> index b8f1dc0761e4b..9826d5fc12e34 100644
>> --- a/arch/x86/include/asm/div64.h
>> +++ b/arch/x86/include/asm/div64.h
>> @@ -71,6 +71,8 @@ static inline u64 mul_u32_u32(u32 a, u32 b)
>>  }
>>  #define mul_u32_u32 mul_u32_u32
>>  
>> +#define __div64_32 /* not needed */
>
> This comment, *after* having read the commit message makes sense.
>
> When you look at it alone, after having opened the file, makes me
> scratch my head and wonder what is that thing supposed to mean. Please
> extend it.
>
> And put the comment ontop, not sideways.

Right, makes sense. How about this version?

--- a/arch/x86/include/asm/div64.h
+++ b/arch/x86/include/asm/div64.h
@@ -71,6 +71,11 @@ static inline u64 mul_u32_u32(u32 a, u32 b)
 }
 #define mul_u32_u32 mul_u32_u32
 
+/*
+ * __div64_32() is never called on x86, so prevent the
+ * generic definition from getting built.
+ */
+#define __div64_32
 
 #else
 # include <asm-generic/div64.h>


       Arnd
  

On Tue, Aug 01, 2023 at 10:48:02PM +0200, Arnd Bergmann wrote:
> Right, makes sense. How about this version?
> 
> --- a/arch/x86/include/asm/div64.h
> +++ b/arch/x86/include/asm/div64.h
> @@ -71,6 +71,11 @@ static inline u64 mul_u32_u32(u32 a, u32 b)
>  }
>  #define mul_u32_u32 mul_u32_u32
>  
> +/*
> + * __div64_32() is never called on x86, so prevent the
> + * generic definition from getting built.
> + */
> +#define __div64_32
>  
>  #else
>  # include <asm-generic/div64.h>

Yap.

Thx.
  

On Wed, 2 Aug 2023, Borislav Petkov wrote:

> > --- a/arch/x86/include/asm/div64.h
> > +++ b/arch/x86/include/asm/div64.h
> > @@ -71,6 +71,11 @@ static inline u64 mul_u32_u32(u32 a, u32 b)
> >  }
> >  #define mul_u32_u32 mul_u32_u32
> >  
> > +/*
> > + * __div64_32() is never called on x86, so prevent the
> > + * generic definition from getting built.
> > + */
> > +#define __div64_32
> >  
> >  #else
> >  # include <asm-generic/div64.h>
> 
> Yap.

 Hmm, shouldn't this be something like:

#define __div64_32(n, base) BUILD_BUG()

instead?

 Otherwise you risk `__div64_32(n, base)' getting expanded to `(n, base)', 
if the macro does get called mistakenly for some reason, and the expansion 
is a valid expression which may not produce a warning even, depending on 
usage.

  Maciej
  

On Mon, Aug 07, 2023 at 09:37:00PM +0100, Maciej W. Rozycki wrote:
>  Otherwise you risk `__div64_32(n, base)' getting expanded to `(n, base)', 

You mean in the very obscure case of a 32-bit kernel where they don't call
do_div() but call this low level function?

I'd say if they can't be bothered to even grep the tree for the right usage,
they deserve both pieces... :)
  

On Mon, 7 Aug 2023, Borislav Petkov wrote:

> On Mon, Aug 07, 2023 at 09:37:00PM +0100, Maciej W. Rozycki wrote:
> >  Otherwise you risk `__div64_32(n, base)' getting expanded to `(n, base)', 
> 
> You mean in the very obscure case of a 32-bit kernel where they don't call
> do_div() but call this low level function?
> 
> I'd say if they can't be bothered to even grep the tree for the right usage,
> they deserve both pieces... :)

 Well, people do make mistakes and life is tough enough already, so why 
make it tougher when we can have a free sanity check?  I mean there's 
hardly any cost from the extra characters added and it can save someone 
hassle with debugging, which is always tough by definition.

 I've suffered from silly mistakes myself on many occasions, possibly from
being distracted in the middle of doing something, and while I figured 
things out eventually, it often cost me a day of effort or so wasted in 
chasing them.

  Maciej