From: Arnd Bergmann <[email protected]>
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 <[email protected]>
---
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 */
+
#else
# include <asm-generic/div64.h>
--
2.39.2
On Tue, Jul 25, 2023 at 03:48:34PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> 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 <[email protected]>
> ---
> 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.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
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 <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
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... :)
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
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