Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scanf cannot parse large unsigned int64s #6316

Closed
vicuna opened this issue Feb 4, 2014 · 6 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link

commented Feb 4, 2014

Original bug ID: 6316
Reporter: seliopou
Assigned to: @gasche
Status: closed (set by @xavierleroy on 2017-02-16T14:15:06Z)
Resolution: fixed
Priority: normal
Severity: minor
Platform: OCaml 4.01.0
OS: Debian
OS Version: wheezy/sid
Version: 4.01.0
Target version: 4.03.0+dev / +beta1
Fixed in version: 4.03.0+dev / +beta1
Category: standard library
Tags: junior_job
Monitored by: @hcarty

Bug description

Any string literal representing an unsigned int64 that is larger than 2^63 cannot be parsed by Scanf.sscanf using the format string "%Lu".

Steps to reproduce

The following code snippet consistently reproduces the issue:

Scanf.sscanf "12306459064359371967" "%Lu" (fun i -> i);;

File attachments

@vicuna

This comment has been minimized.

Copy link
Author

commented May 12, 2015

Comment author: @damiendoligez

@gasche: shouldn't we postpone this one to 4.03?

@vicuna

This comment has been minimized.

Copy link
Author

commented May 12, 2015

Comment author: @gasche

Reasonable. I will postpone it so that it doesn't block the release, but if a patch comes in time, and is safe, I will be tempted to merge it in 4.02.

@vicuna

This comment has been minimized.

Copy link
Author

commented May 13, 2015

Comment author: bvaugon

I attatch two patches that fix the problem with two different solutions :

  • Version 1: OCaml implementation of unsigned_{int,int32,int64,nativeint}_of_string
    • Pure OCaml (improved portability)
    • Faster than C implem (surprisingly, or not ^^)
    • Code replication to manage int, int32, int64 and nativeint
    • Increase scanf.ml code size
  • Version 2: extend int_of_string functionnality with "0u1234" syntax (for unsigned ints)
    • Little code modification
    • Extend int_of_string functionalities
    • A bit slower and less readable

Any opinion?

@vicuna

This comment has been minimized.

Copy link
Author

commented May 13, 2015

Comment author: @gasche

The first patch is too invasive for 4.02, but could be an option for 4.03 (but it's not clear to me why we would write the unsigned parsing functions in OCaml, and keep the signed parsing in C). The second patch seems reasonable, I'd like the opinion of our release manager.

@vicuna

This comment has been minimized.

Copy link
Author

commented May 13, 2015

Comment author: bvaugon

In fact, since the OCaml implementation of int_of_string'like functions seems to be (between 1.5 and 2 times) faster than the current C implementation (on some machines, maybe not anywhere), it might be interresting (also for portability, homogeneity and simplicity) to encode all of them in OCaml.

The first version principally adds code, and contains modifications of only few lines of the existing ml code. IMHO, it seems safer to release it than the second version that modify the existing C code, except if the syntax "0u..." is considered interesting for int_of_string.

I forgot to tell that this patch also fix "%u", "%lu" and "%nu".

@vicuna

This comment has been minimized.

Copy link
Author

commented Jul 24, 2015

Comment author: @xavierleroy

Fixed in commit 16241. I came up independently with the second approach suggested by bvaugon, so it must be the right approach :-) I didn't feel the need to reject int_of_string "-0u123", because we already have a tolerance for int_of_string "-0x123".

@vicuna vicuna closed this Feb 16, 2017

@vicuna vicuna added this to the 4.03.0 milestone Mar 14, 2019

@vicuna vicuna added the bug label Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.