arch/x86/lib/misc.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
In num_digits(), the negation of the input value "val = -val"
causes undefined behavior when val is INT_MIN, as its absolute
value cannot be represented as a signed 32-bit integer.
This leads to incorrect results (returning 2 instead of 11).
By promoting the value to long long before negation, we ensure
the absolute value is correctly handled.
Signed-off-by: David Desobry <david.desobry@formalgen.com>
---
arch/x86/lib/misc.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/x86/lib/misc.c b/arch/x86/lib/misc.c
index 40b81c338ae5..c975db6ccb9f 100644
--- a/arch/x86/lib/misc.c
+++ b/arch/x86/lib/misc.c
@@ -8,15 +8,16 @@
*/
int num_digits(int val)
{
+ long long v = val;
long long m = 10;
int d = 1;
- if (val < 0) {
+ if (v < 0) {
d++;
- val = -val;
+ v = -v;
}
- while (val >= m) {
+ while (v >= m) {
m *= 10;
d++;
}
--
2.43.0
On January 20, 2026 1:42:58 AM PST, David Desobry <david.desobry@formalgen.com> wrote:
>In num_digits(), the negation of the input value "val = -val"
>causes undefined behavior when val is INT_MIN, as its absolute
>value cannot be represented as a signed 32-bit integer.
>
>This leads to incorrect results (returning 2 instead of 11).
>By promoting the value to long long before negation, we ensure
>the absolute value is correctly handled.
>
>Signed-off-by: David Desobry <david.desobry@formalgen.com>
>---
> arch/x86/lib/misc.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
>diff --git a/arch/x86/lib/misc.c b/arch/x86/lib/misc.c
>index 40b81c338ae5..c975db6ccb9f 100644
>--- a/arch/x86/lib/misc.c
>+++ b/arch/x86/lib/misc.c
>@@ -8,15 +8,16 @@
> */
> int num_digits(int val)
> {
>+ long long v = val;
> long long m = 10;
> int d = 1;
>
>- if (val < 0) {
>+ if (v < 0) {
> d++;
>- val = -val;
>+ v = -v;
> }
>
>- while (val >= m) {
>+ while (v >= m) {
> m *= 10;
> d++;
> }
That has got to be the dumbest possible implementation of that task, bug or no bug.
A switch statement would be simpler and faster.
Fair point! I've sent a v2 that replaces the loop with a switch
statement (using GCC ranges). It's faster, handles INT_MIN properly via
an unsigned cast, and I've cleaned up that mobile-submission comment
while I was at it.
Le 20/01/2026 à 17:23, H. Peter Anvin a écrit :
> On January 20, 2026 1:42:58 AM PST, David Desobry <david.desobry@formalgen.com> wrote:
>> In num_digits(), the negation of the input value "val = -val"
>> causes undefined behavior when val is INT_MIN, as its absolute
>> value cannot be represented as a signed 32-bit integer.
>>
>> This leads to incorrect results (returning 2 instead of 11).
>> By promoting the value to long long before negation, we ensure
>> the absolute value is correctly handled.
>>
>> Signed-off-by: David Desobry <david.desobry@formalgen.com>
>> ---
>> arch/x86/lib/misc.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/lib/misc.c b/arch/x86/lib/misc.c
>> index 40b81c338ae5..c975db6ccb9f 100644
>> --- a/arch/x86/lib/misc.c
>> +++ b/arch/x86/lib/misc.c
>> @@ -8,15 +8,16 @@
>> */
>> int num_digits(int val)
>> {
>> + long long v = val;
>> long long m = 10;
>> int d = 1;
>>
>> - if (val < 0) {
>> + if (v < 0) {
>> d++;
>> - val = -val;
>> + v = -v;
>> }
>>
>> - while (val >= m) {
>> + while (v >= m) {
>> m *= 10;
>> d++;
>> }
> That has got to be the dumbest possible implementation of that task, bug or no bug.
>
> A switch statement would be simpler and faster.
On Tue, 20 Jan 2026 18:48:54 +0100 David Desobry <david.desobry@formalgen.com> wrote: > Fair point! I've sent a v2 that replaces the loop with a switch > statement (using GCC ranges). It's faster, Look at the 'crap' that clang generates. And remember that mispredicted branches are expensive and the default is either 'random', 'not taken' or 'backwards branches taken'. It is likely better to have a lot of not-taken branches than a binary tree. David
On Tue, 20 Jan 2026 08:23:16 -0800
"H. Peter Anvin" <hpa@zytor.com> wrote:
> On January 20, 2026 1:42:58 AM PST, David Desobry <david.desobry@formalgen.com> wrote:
> >In num_digits(), the negation of the input value "val = -val"
> >causes undefined behavior when val is INT_MIN, as its absolute
> >value cannot be represented as a signed 32-bit integer.
> >
> >This leads to incorrect results (returning 2 instead of 11).
> >By promoting the value to long long before negation, we ensure
> >the absolute value is correctly handled.
> >
> >Signed-off-by: David Desobry <david.desobry@formalgen.com>
> >---
> > arch/x86/lib/misc.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> >diff --git a/arch/x86/lib/misc.c b/arch/x86/lib/misc.c
> >index 40b81c338ae5..c975db6ccb9f 100644
> >--- a/arch/x86/lib/misc.c
> >+++ b/arch/x86/lib/misc.c
> >@@ -8,15 +8,16 @@
> > */
> > int num_digits(int val)
> > {
> >+ long long v = val;
> > long long m = 10;
> > int d = 1;
> >
> >- if (val < 0) {
> >+ if (v < 0) {
> > d++;
> >- val = -val;
> >+ v = -v;
> > }
> >
> >- while (val >= m) {
> >+ while (v >= m) {
> > m *= 10;
> > d++;
> > }
>
> That has got to be the dumbest possible implementation of that task, bug or no bug.
And you really don't want to be doing 64bit maths on a 32bit system.
> A switch statement would be simpler and faster.
I think you mean a chain of if statement - you'd need a lot of them.
But you could have:
if (val < 0) {
if (val < -999999999)
return 11;
val = -val;
d++;
} else {
if (val > 999999999)
return 10;
}
then use whatever scheme looks best.
David
On Tue, Jan 20, 2026 at 08:23:16AM -0800, H. Peter Anvin wrote:
> That has got to be the dumbest possible implementation of that task, bug or no bug.
LOL, did you read the comment above that function?
ROTFL.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On January 20, 2026 8:40:50 AM PST, Borislav Petkov <bp@alien8.de> wrote: >On Tue, Jan 20, 2026 at 08:23:16AM -0800, H. Peter Anvin wrote: >> That has got to be the dumbest possible implementation of that task, bug or no bug. > >LOL, did you read the comment above that function? > >ROTFL. > I did. No wonder it sucked! 🤣
On January 20, 2026 10:02:06 AM PST, "H. Peter Anvin" <hpa@zytor.com> wrote:
>On January 20, 2026 8:40:50 AM PST, Borislav Petkov <bp@alien8.de> wrote:
>>On Tue, Jan 20, 2026 at 08:23:16AM -0800, H. Peter Anvin wrote:
>>> That has got to be the dumbest possible implementation of that task, bug or no bug.
>>
>>LOL, did you read the comment above that function?
>>
>>ROTFL.
>>
>
>I did. No wonder it sucked! 🤣
Seriously, though. As Linus likes to point out, there is a huge difference between "this is stupid" and "you are stupid."
I have said "this is stupid" about my own code more times than I can count. For good reason.
To be honest, I didn't even need to see the comment to know that that was probably my code in the first place. I recognized my own style at a glance; in particular the kind of code I tend to write specifically for the purpose of small as opposed to fast code. I remember writing this code while waiting for a table in a restaurant :) but I don't remember the context.
I was in a hurry but I almost put in a snark saying "this must be my code or something." :)
Anyway, I do not mind anyone calling my code stupid, especially if it actually is. It may or may not have been stupid in the first place, or the context might have changed, but it doesn't really matter — replacing stupid code is how we improve Linux :)
-hpa (proud author of plenty of stupid code)
On Tue, Jan 20, 2026 at 10:17:09AM -0800, H. Peter Anvin wrote:
> Seriously, though. As Linus likes to point out, there is a huge difference
> between "this is stupid" and "you are stupid."
Yap.
> I have said "this is stupid" about my own code more times than I can count.
> For good reason.
Same here.
> To be honest, I didn't even need to see the comment to know that that was
> probably my code in the first place. I recognized my own style at a glance;
> in particular the kind of code I tend to write specifically for the purpose
> of small as opposed to fast code. I remember writing this code while waiting
> for a table in a restaurant :) but I don't remember the context.
You sent it and I productized it into a patch by saying:
Change num_digits() to hpa's division-avoiding, cell-phone-typed
version which he went at great lengths and pains to submit on a
Saturday evening.
> Anyway, I do not mind anyone calling my code stupid, especially if it
> actually is. It may or may not have been stupid in the first place, or the
> context might have changed, but it doesn't really matter — replacing stupid
> code is how we improve Linux :)
Absolutely. We do that all the time and it used to work back then. Hell, it
works fine for CPU numbers but if someone wants to give it INT_MIN... we
didn't care about that. We needed it for this gunk:
a17bce4d1dce ("x86/boot: Further compress CPUs bootup message")
Which reminds me:
@David, are you staring at the code or are you using this function somewhere
and hitting this corner case?
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 2026-01-20 11:16, Borislav Petkov wrote:
>
> Absolutely. We do that all the time and it used to work back then. Hell, it
> works fine for CPU numbers but if someone wants to give it INT_MIN... we
> didn't care about that. We needed it for this gunk:
>
> a17bce4d1dce ("x86/boot: Further compress CPUs bootup message")
>
For negative numbers? Dave Hansen pointed out that we currently don't ever
pass negative numbers, in which case it would make more sense to have the
function simply take an unsigned int.
-hpa
On Tue, Jan 20, 2026 at 11:50:41AM -0800, H. Peter Anvin wrote:
> For negative numbers? Dave Hansen pointed out that we currently don't ever
> pass negative numbers, in which case it would make more sense to have the
> function simply take an unsigned int.
Yes, that's why I'm asking what we're even fixing here...
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Tue, 20 Jan 2026 21:13:46 +0100 Borislav Petkov <bp@alien8.de> wrote: > On Tue, Jan 20, 2026 at 11:50:41AM -0800, H. Peter Anvin wrote: > > For negative numbers? Dave Hansen pointed out that we currently don't ever > > pass negative numbers, in which case it would make more sense to have the > > function simply take an unsigned int. > > Yes, that's why I'm asking what we're even fixing here... You can certainly just document it doesn't work for INT_MIN. David
© 2016 - 2026 Red Hat, Inc.