Opened 5 years ago

Closed 9 months ago

Last modified 9 months ago

#10909 closed bug (invalid)

[easy] strtol doesn't handle overflow properly

Reported by: pulkomandy Owned by: nobody
Priority: normal Milestone: R1
Component: System/POSIX Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

When trying to parse a number too big to fit a long, strtol returns 0 instead of LONG_MAX. If I read the POSIX spec correctly, it should:

  • Set end to the character after the last digit in the string
  • Set errno to ERANGE
  • Return LONG_MAX.

See attached test program.

http://pubs.opengroup.org/onlinepubs/007904875/functions/strtol.html

Attachments (1)

test2.c (105 bytes ) - added by pulkomandy 5 years ago.
Test program that shows strtol returning 0.

Download all attachments as: .zip

Change History (11)

by pulkomandy, 5 years ago

Attachment: test2.c added

Test program that shows strtol returning 0.

comment:1 by luroh, 5 years ago

Milestone: R1Unscheduled

Move POSIX compatibility related tickets out of R1 milestone (FutureHaiku/Features).

comment:2 by PtrNull, 9 months ago

I am working on it

comment:3 by waddlesplash, 9 months ago

We should probably just use Musl's version rather than writing our own.

comment:4 by PtrNull, 9 months ago

Well, has the problem been solved?

comment:5 by pulkomandy, 9 months ago

Milestone: UnscheduledR1

Probably not (you can check the attached test program), but rather than fixing the existing code, we can try to get the implementation from https://www.musl-libc.org/ and see if it can be used to replace ours. It's likely that the implementation in musl is not affected by this bug.

We could also try to get an implementation from FreeBSD or other places where this function is implemented.

comment:6 by PtrNull, 9 months ago

Thank you, I will try to solve it.

comment:7 by PtrNull, 9 months ago

I think this is not a bug, but a problem with the test program. Because this function is not declared in stdio.h, its function declaration will default to int (...), so you can compile it. When linking, it will link to the standard library by default, so the link will pass. I didn't check the standard, but I can imagine that this is an undefined behavior or an unspecified behavior, and the compiler also gives the correct warning.

comment:8 by pulkomandy, 9 months ago

int and long int are the same on 32-bit Haiku. No undefined behavior here. You can add the include if you want.

The function should not return 0, but LONG_MAX. Does it?

comment:9 by PtrNull, 9 months ago

Of course yes.

#include "stdio.h"
#include "stdlib.h"
int main(void){
    char *end[50];
    long u = strtol("99999999999999999999999", end, 10);
    printf("u=%ld\n", u);
    printf("LONG_MAX=%ld\n", LONG_MAX);
    return 0;
}

Return LONG_MAX correctly.

comment:10 by pulkomandy, 9 months ago

Resolution: invalid
Status: newclosed

Uh, indeed. The problem is not the return type, but the missing arguments. Of course it works better if you call it with all 3 arguments. Sorry!

Thanks for investigating!

Last edited 9 months ago by pulkomandy (previous) (diff)
Note: See TracTickets for help on using tickets.