Bugs with parsing BigInt, BigFloat, and Decimals (from DecFP.jl) when no \0 immediately past end

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Bugs with parsing BigInt, BigFloat, and Decimals (from DecFP.jl) when no \0 immediately past end

Scott Jones
In the course of adding support to CSV.jl for being able to load into arbitrary `AbstractFloat` types, via a keyword argument `floattype` (which defaults to `Float64`),
I ran into a number of bugs (both in v0.4.x and v0.5-dev).  The problem occurs because pointer_to_string() does not create a string that has a terminating `\0` (which I feel is correct),
but `Cstring` does not ensure that a string passed to C has a terminating `\0`.
This is inconsistent with `Cwstring`, which *does* make sure that a `Cwstring` does have a terminating `Cwchar(0)`.

I'd make a PR to fix these bugs, but since I'm unable to, I'm posting the issue here.

Here are some test cases demonstrating the bugs:

julia> s = "53.9abcdef"

"53.9abcdef"


julia> u = pointer_to_string(pointer(s.data),2)

"53"


julia> t = pointer_to_string(pointer(s.data),4)

"53.9"


julia> parse(Dec64, t)

ERROR: ArgumentError: invalid number format 53.9

 in parse at /j/DecFP.jl/src/DecFP.jl:82


julia> parse(Dec64, u)

ERROR: ArgumentError: invalid number format 53

 in parse at /j/DecFP.jl/src/DecFP.jl:82


julia> parse(BigInt, u)

ERROR: ArgumentError: invalid BigInt: "53"

 in tryparse_internal at gmp.jl:104

 in parse at parse.jl:146


julia> parse(BigFloat, t)

ERROR: ArgumentError: invalid number format "53.9" for BigFloat

 in parse at parse.jl:164


Reply | Threaded
Open this post in threaded view
|

Re: Bugs with parsing BigInt, BigFloat, and Decimals (from DecFP.jl) when no \0 immediately past end

Tamas Papp
On Fri, May 20 2016, Scott Jones wrote:

> I'd make a PR to fix these bugs, but since I'm unable to, I'm posting the
> issue here.

Creating an issue on Github might be useful.

Best,

Tamas
Reply | Threaded
Open this post in threaded view
|

Re: Bugs with parsing BigInt, BigFloat, and Decimals (from DecFP.jl) when no \0 immediately past end

Ismael Venegas Castelló

That's a problem (again).

Scott has been baned for over a year from github, yet he has still kept contributing to Julia in numerous ways and polishing his social skills as he, me and others can prove.

I don't want to discuss about his banning or if he deserved it. What is done is done.

I think it is fair to say that it's time to at least reconsider releasing this ban on him and I mean as a community and in an open maner (specially now that JuliaCon is comming and he will be there with you guys) IMO.

I'd like you to know he has my support as I think he is a valuable asset to our community. I hope we can reach consensus and likewise find a way, ...again, as acommunity, to deliberate about banning anyone or not in the future.

Our community means awhile lot to me, please let me know if I can help in any way to help us achieve this and where should we continue this discussion.

Thanks in advance.

On May 21, 2016 1:18 AM, "Tamas Papp" <[hidden email]> wrote:
On Fri, May 20 2016, Scott Jones wrote:

> I'd make a PR to fix these bugs, but since I'm unable to, I'm posting the
> issue here.

Creating an issue on Github might be useful.

Best,

Tamas
Reply | Threaded
Open this post in threaded view
|

Re: Bugs with parsing BigInt, BigFloat, and Decimals (from DecFP.jl) when no \0 immediately past end

Tom Breloff
I'd like to officially throw my support for lifting any ban. I didn't follow the situation so I don't actually know what preceded it, but I've met, talked, and collaborated with Scott and I think he's a valuable member of the community that is trying his hardest to make Julia great. 

On Saturday, May 21, 2016, Ismael Venegas Castelló <[hidden email]> wrote:

That's a problem (again).

Scott has been baned for over a year from github, yet he has still kept contributing to Julia in numerous ways and polishing his social skills as he, me and others can prove.

I don't want to discuss about his banning or if he deserved it. What is done is done.

I think it is fair to say that it's time to at least reconsider releasing this ban on him and I mean as a community and in an open maner (specially now that JuliaCon is comming and he will be there with you guys) IMO.

I'd like you to know he has my support as I think he is a valuable asset to our community. I hope we can reach consensus and likewise find a way, ...again, as acommunity, to deliberate about banning anyone or not in the future.

Our community means awhile lot to me, please let me know if I can help in any way to help us achieve this and where should we continue this discussion.

Thanks in advance.

On May 21, 2016 1:18 AM, "Tamas Papp" <<a href="javascript:_e(%7B%7D,&#39;cvml&#39;,&#39;tkpapp@gmail.com&#39;);" target="_blank">tkpapp@...> wrote:
On Fri, May 20 2016, Scott Jones wrote:

> I'd make a PR to fix these bugs, but since I'm unable to, I'm posting the
> issue here.

Creating an issue on Github might be useful.

Best,

Tamas
Reply | Threaded
Open this post in threaded view
|

Re: Bugs with parsing BigInt, BigFloat, and Decimals (from DecFP.jl) when no \0 immediately past end

Tamas Papp
In reply to this post by Ismael Venegas Castelló
Sorry, I was not aware of the ban, now I understand why the bug report
came to the mailing list.

Still, perhaps someone who is not banned could report the issue,
otherwise it could get lost in traffic (discussion is already going in
another direction).

On Sat, May 21 2016, Ismael Venegas Castelló wrote:

> That's a problem (again).
>
> Scott has been baned for over a year from github, yet he has still kept
> contributing to Julia in numerous ways and polishing his social skills as
> he, me and others can prove.
>
> I don't want to discuss about his banning or if he deserved it. What is
> done is done.
>
> I think it is fair to say that it's time to at least reconsider releasing
> this ban on him and I mean as a community and in an open maner (specially
> now that JuliaCon is comming and he will be there with you guys) IMO.
>
> I'd like you to know he has my support as I think he is a valuable asset to
> our community. I hope we can reach consensus and likewise find a way,
> ...again, as acommunity, to deliberate about banning anyone or not in the
> future.
>
> Our community means awhile lot to me, please let me know if I can help in
> any way to help us achieve this and where should we continue this
> discussion.
>
> Thanks in advance.
> On May 21, 2016 1:18 AM, "Tamas Papp" <[hidden email]> wrote:
>
>> On Fri, May 20 2016, Scott Jones wrote:
>>
>> > I'd make a PR to fix these bugs, but since I'm unable to, I'm posting the
>> > issue here.
>>
>> Creating an issue on Github might be useful.
>>
>> Best,
>>
>> Tamas
>>

Reply | Threaded
Open this post in threaded view
|

Re: Bugs with parsing BigInt, BigFloat, and Decimals (from DecFP.jl) when no \0 immediately past end

Ismael Venegas Castelló
In reply to this post by Scott Jones
I have created the issue at the Julia repo, just copy and paste verbatim:

* https://github.com/JuliaLang/julia/issues/16499


El viernes, 20 de mayo de 2016, 16:31:49 (UTC-5), Scott Jones escribió:
In the course of adding support to CSV.jl for being able to load into arbitrary `AbstractFloat` types, via a keyword argument `floattype` (which defaults to `Float64`),
I ran into a number of bugs (both in v0.4.x and v0.5-dev).  The problem occurs because pointer_to_string() does not create a string that has a terminating `\0` (which I feel is correct),
but `Cstring` does not ensure that a string passed to C has a terminating `\0`.
This is inconsistent with `Cwstring`, which *does* make sure that a `Cwstring` does have a terminating `Cwchar(0)`.

I'd make a PR to fix these bugs, but since I'm unable to, I'm posting the issue here.

Here are some test cases demonstrating the bugs:

julia> s = "53.9abcdef"

"53.9abcdef"


julia> u = pointer_to_string(pointer(s.data),2)

"53"


julia> t = pointer_to_string(pointer(s.data),4)

"53.9"


julia> parse(Dec64, t)

ERROR: ArgumentError: invalid number format 53.9

 in parse at /j/DecFP.jl/src/DecFP.jl:82


julia> parse(Dec64, u)

ERROR: ArgumentError: invalid number format 53

 in parse at /j/DecFP.jl/src/DecFP.jl:82


julia> parse(BigInt, u)

ERROR: ArgumentError: invalid BigInt: "53"

 in tryparse_internal at gmp.jl:104

 in parse at parse.jl:146


julia> parse(BigFloat, t)

ERROR: ArgumentError: invalid number format "53.9" for BigFloat

 in parse at parse.jl:164


Reply | Threaded
Open this post in threaded view
|

Re: Bugs with parsing BigInt, BigFloat, and Decimals (from DecFP.jl) when no \0 immediately past end

Scott Jones
In reply to this post by Tom Breloff
Thanks for the kind words!
I must say, Julia is already great, if it *weren't* so great, I wouldn't have been such a pest trying to fix the few issues I've found (like the one this post was about) ;-)
It *is* getting better and better (although I'm still rather impatient for some of the stuff described at JuliaCon 2015), and I've never met a more passionate community.

Best to everybody!

On Saturday, May 21, 2016 at 8:41:39 AM UTC-4, Tom Breloff wrote:
I'd like to officially throw my support for lifting any ban. I didn't follow the situation so I don't actually know what preceded it, but I've met, talked, and collaborated with Scott and I think he's a valuable member of the community that is trying his hardest to make Julia great. 

On Saturday, May 21, 2016, Ismael Venegas Castelló <<a href="javascript:" target="_blank" gdf-obfuscated-mailto="dLn830CgAgAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">ismael...@...> wrote:

That's a problem (again).

Scott has been baned for over a year from github, yet he has still kept contributing to Julia in numerous ways and polishing his social skills as he, me and others can prove.

I don't want to discuss about his banning or if he deserved it. What is done is done.

I think it is fair to say that it's time to at least reconsider releasing this ban on him and I mean as a community and in an open maner (specially now that JuliaCon is comming and he will be there with you guys) IMO.

I'd like you to know he has my support as I think he is a valuable asset to our community. I hope we can reach consensus and likewise find a way, ...again, as acommunity, to deliberate about banning anyone or not in the future.

Our community means awhile lot to me, please let me know if I can help in any way to help us achieve this and where should we continue this discussion.

Thanks in advance.

On May 21, 2016 1:18 AM, "Tamas Papp" wrote:
On Fri, May 20 2016, Scott Jones wrote:

> I'd make a PR to fix these bugs, but since I'm unable to, I'm posting the
> issue here.

Creating an issue on Github might be useful.

Best,

Tamas
Reply | Threaded
Open this post in threaded view
|

Re: Bugs with parsing BigInt, BigFloat, and Decimals (from DecFP.jl) when no \0 immediately past end

Scott Jones
In reply to this post by Tamas Papp
I'm busy at the ODSC East conference today (Stefan gave a great keynote talk, first of 4, cool that one of Julia's co-creators got first spot ;-) )
I'm working on some PRs for Steven Johnson's DecFP.jl to work around this for DecFP (and to fix some other things broken by recent v0.5 changes),

I can investigate further fixes for the BigInt and BigFloat problems, and then if somebody else can create PRs from my branches, then they can be merged (Jeff, Tony and some others have been very helpful pulling in some of my fixes over the last 5+ months since the ban).

I think the take-away from these bugs I found is people need to be very careful to make sure that Cstring is used now instead of Ptr{UInt8} (and get Cstring fixed to ensure a terminating \0),
we may need to review all ccall's in Base passing strings.

-Scott

On Saturday, May 21, 2016 at 8:53:45 AM UTC-4, Tamas Papp wrote:
Sorry, I was not aware of the ban, now I understand why the bug report
came to the mailing list.

Still, perhaps someone who is not banned could report the issue,
otherwise it could get lost in traffic (discussion is already going in
another direction).

On Sat, May 21 2016, Ismael Venegas Castelló wrote:

> That's a problem (again).
>
> Scott has been baned for over a year from github, yet he has still kept
> contributing to Julia in numerous ways and polishing his social skills as
> he, me and others can prove.
>
> I don't want to discuss about his banning or if he deserved it. What is
> done is done.
>
> I think it is fair to say that it's time to at least reconsider releasing
> this ban on him and I mean as a community and in an open maner (specially
> now that JuliaCon is comming and he will be there with you guys) IMO.
>
> I'd like you to know he has my support as I think he is a valuable asset to
> our community. I hope we can reach consensus and likewise find a way,
> ...again, as acommunity, to deliberate about banning anyone or not in the
> future.
>
> Our community means awhile lot to me, please let me know if I can help in
> any way to help us achieve this and where should we continue this
> discussion.
>
> Thanks in advance.
> On May 21, 2016 1:18 AM, "Tamas Papp" <<a href="javascript:" target="_blank" gdf-obfuscated-mailto="qHoYD-qgAgAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">tkp...@...> wrote:
>
>> On Fri, May 20 2016, Scott Jones wrote:
>>
>> > I'd make a PR to fix these bugs, but since I'm unable to, I'm posting the
>> > issue here.
>>
>> Creating an issue on Github might be useful.
>>
>> Best,
>>
>> Tamas
>>