Skip to content

/go

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

net: internal dtoi and xtoi helpers have unusual API #16350

Closed
mdempsky opened this issue Jul 13, 2016 · 3 comments

Comments

Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
  Go1.8
3 participants
@mdempsky @bradfitz @gopherbot
@mdempsky
Copy link
Member

commented Jul 13, 2016 •
edited

These internal helpers have signatures like:

func dtoi(s string, i0 int) ...
func xtoi(s string, i0 int) ...

and they parse an integer value from &s[i0]. Especially since the majority of callers use i0==0, it seems like it would be simpler / more idiomatic to just have the caller pass s[i0:] as an argument.

In particular, dtoi theoretically supports parsing negative integers, except it looks for the '-' character at s[0], not s[i0].

/cc @bradfitz @dpiddy

@mdempsky mdempsky added this to the Go1.8 milestone Jul 13, 2016

@bradfitz

This comment has been minimized.

Sign in to view
Copy link
Member

commented Jul 13, 2016

Yes, those weird me out every time I encounter them.
@gopherbot

This comment has been minimized.

Sign in to view
Copy link

commented Aug 17, 2016

CL https://golang.org/cl/27206 mentions this issue.

@gopherbot gopherbot closed this in 00b779a Aug 17, 2016

@gopherbot

This comment has been minimized.

Sign in to view
Copy link

commented Sep 2, 2016

CL https://golang.org/cl/28414 mentions this issue.

gopherbot pushed a commit that referenced this issue Sep 7, 2016

@dsnet
net: remove parsing of negative decimals in IPv4 literal 
https://golang.org/cl/27206 fixed the dtoi function such that
it now properly parses negative number. Ironically, this causes
several other functions that depended on dtoi to now (incorrectly)
parse negative numbers.

For example, ParseCIDR("-1.0.0.0/32") used to be rejected prior to the
above CL, but is now accepted even though it is an invalid CIDR notation.
This CL fixes that regression.

We fix this by removing the signed parsing logic entirely from dtoi.
It was introduced relatively recently in https://golang.org/cl/12447
to fix a bug where an invalid port was improperly being parsed as OK.
It seems to me that the fix in that CL to the port handling logic was
sufficient such that a change to dtoi was unnecessary.

Updates #16350

Change-Id: I414bb1aa27d0a226ebd4b05a09cb40d784691b43
Reviewed-on: https://go-review.googlesource.com/28414
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
3ef0e8f

@golang golang locked and limited conversation to collaborators Sep 2, 2017

@gopherbot gopherbot added the FrozenDueToAge label Sep 2, 2017

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