Wed 04 May 2016 02:51:43 PM UTC, comment #12:
Yeah, I guess my thought was any test we add for 64-bit numbers is going to exceed the range of what we can do right now, so this is a quick and dirty inaccurate test, we can fix later once we get bug #45945 fixed.
|
Tue 26 Apr 2016 05:59:42 PM UTC, comment #11:
Maybe one could (additionally?) test for
which will make more sense once bug #45945 should be solved.
|
Thu 21 Apr 2016 07:40:23 PM UTC, comment #10:
I went with the closest test I could think of that is still accurate and still somewhat simple to write.
I wanted to keep the value to 64 bits, but we know it evaluates to a double value, so that test should be accurate within the known constraints of the Octave interpreter.
Maybe 0xFFFF_FFFF_FFFF_0000 would be a more accurate test?
|
Thu 21 Apr 2016 07:33:30 AM UTC, comment #9:
Thank you for pushing the patch.
Wrt the BIST tests. They do not fail due to double precission but:
So in fact, they are one off. Is it worth asserting the "more correct" result there?
|
Wed 20 Apr 2016 10:33:35 PM UTC, comment #8:
I pushed your updated patch, and followed up with some simple tests in parser.tst:
http://hg.savannah.gnu.org/hgweb/octave/rev/986dbd769bb1
http://hg.savannah.gnu.org/hgweb/octave/rev/a3a412dee704
Works for me on Debian 32-bit and 64-bit. Thanks for your contribution!
|
Wed 20 Apr 2016 09:04:03 AM UTC, comment #7:
Thank you, Mike, for your feedback.
I agree with you that the changed handling of large decimal integer input is wrong. Since all input is cast to double anyway (in Octave and in Matlab), I should have stripped off that part of the patch from the beginning.
The fact that integer input seems to not loose precission in Matlab when used for some functions is the other bug #45945. The uintmax_t overflow has to be handled there.
I did not find the patch with your changes so I'll attach a new one that does not touch the parsing of decimal numbers and that uses uintmax_t for the binary cast.
I will file a new bug for the related issue with the scanf family as you suggested.
(file #36970)
|
Tue 19 Apr 2016 07:48:10 PM UTC, comment #6:
This patch looks good for the most part, I tweaked a couple minor style things, and used uintmax_t for the cast in the binary number case. I've tested on Debian with a 32-bit build and it does correct the hex input case that is also seen on Windows.
But one big thing that this patch changes is the handling of integer values that are larger than 2^64. Previous to this patch,
Now with this patch applied it limits integer inputs to 2^64.
I think this patch should not affect decimal number input based on this, but I'm not sure.
What does Matlab do with
|
Tue 19 Apr 2016 06:51:37 PM UTC, comment #5:
Let's put the scanf issue aside as unrelated for the moment, since you have a fix for this but not that, I think it should be reported and handled separately. But just to note, Octave explicitly says in the section "Input Conversion Syntax" that the type modifier is ignored by the scanf family of functions. So basically I think the right direction for that error would be to always read the widest possible value, and add more characters to be silently ignored (including "ll" and "j")
|
Mon 18 Apr 2016 06:08:23 PM UTC, comment #4:
For the similar issue with "sscanf" described in #comment 1, I am unsure what to do.
One way to go would be to allow all C99 modifiers that can be used with scanf (see e.g. on [1]). That would lead to native results on all platforms. However, results would differ for some integer types on different platforms:
"%lx" would result in a 32bit unsigned integer on 32bit Linux and 32/64bit Windows, but in 64bit unsigned integer on 64bit Linux.
Another way would be to "fix" each modifier to a certain length, e.g.:
- "l" --> always 64 bits (Is "long long int" always 64bit on all platforms?)
- "h" --> always 16 bits
- no modifier --> always 32 bits
That would give consistent results on different platforms, but would need some conversion of modifiers for certain (or for the sake of simplicity for all?) platforms (convert "l" to "ll").
The same goes for unsigned integers.
[1] http://www.cplusplus.com/reference/cstdio/scanf/
|
Mon 18 Apr 2016 04:08:41 PM UTC, comment #3:
This patch is part of the patch I originally posted at bug #45945. This one here should be way saver to accept than anything I could come up with foor the other bug.
The laurels should go to Mike Miller who pointed on the right spots there (and to the initial hint of jwe here).
Without this patch on Windows:
With this patch on Windows:
(file #36958)
|
Thu 14 Apr 2016 04:39:57 PM UTC, comment #2:
The problem is in oct-parse.in.yy, where a scanf call is used to convert the text representation ("0x..." hex) to a number. It uses long int, but on Windows that is just 32 bits wide.
It also ultimately converts the result to a double and I'm not sure that's the right thing to do in all cases (see also bug #45945).
|
Thu 14 Apr 2016 01:17:47 PM UTC, comment #1:
I can add that hex2dec works as expected, while sscanf does not: when reading a hex string longer than 8 chars it returns [](0x1), while it works with strings shorter than 9 chars.
|
Thu 14 Apr 2016 12:55:12 AM UTC, original submission:
Francesco Potortì reported on the Octave help list:
http://octave.1599824.n4.nabble.com/Octave-64-bit-for-Windows-can-t-parse-long-hex-integers-td4676253.html
Here I have copied and pasted from Francesco Potortì's post
On Debian:
On Windows:
In practice, when parsing a hex literal bigger than 2^32-1, octave reads
it as 0xffffffff.
|