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

fmt: Scanf EOF error inconsistency #16563

Open
fjl opened this issue Aug 1, 2016 · 7 comments
Open

fmt: Scanf EOF error inconsistency #16563

fjl opened this issue Aug 1, 2016 · 7 comments
Labels
Milestone

Comments

@fjl
Copy link

@fjl fjl commented Aug 1, 2016

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version devel +ff227b8 Thu Jul 21 01:04:22 2016 +0000 linux/amd64
but older versions of Go are also affected.

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN="/home/fjl/bin"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/fjl/"
GORACE=""
GOROOT="/usr/lib/go"
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build153025883=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"

What did you do?

I use fmt.Sscanf to parse integers and big integers.

https://play.golang.org/p/aZVlPM7haY

What did you expect to see?

When there is not enough input Sscanf (and Fscanf, Scanf) should return io.ErrUnexpectedEOF

What did you see instead?

It returns io.ErrUnexpectedEOF for big.Int and io.EOF for uint.

@fjl fjl changed the title fmt: Scanf error inconsistency fmt: Scanf EOF error inconsistency Aug 1, 2016
@quentinmit quentinmit added this to the Go1.8Maybe milestone Aug 1, 2016
@quentinmit quentinmit added the NeedsFix label Oct 10, 2016
@rsc rsc modified the milestones: Go1.9, Go1.8Maybe Nov 2, 2016
@2opremio
Copy link

@2opremio 2opremio commented Dec 15, 2016

Similarly, Scanf is inconsistent when providing a prefix in the format

// results in io.EOF error
_, err = fmt.Sscanf("", "%d\n", &foo)
// results in io.ErrUnexpectedEOF
_, err := fmt.Sscanf("", "prefix %d\n", &foo)

I also expect io.EOF in the latter

https://play.golang.org/p/fNU3lL-KWn

(Happy to create a different issue if you want to treat this separately)

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jun 8, 2017

@robpike, opinions here? Go 1.9, document, do nothing, punt to 1.10?

@robpike
Copy link
Contributor

@robpike robpike commented Jun 8, 2017

Punt to 1.10. Also @rsc was the last one to work on that code, although the problems could well be original. Scanf should never have been written. Mea culpa.

@bradfitz bradfitz modified the milestones: Go1.10, Go1.9 Jun 8, 2017
@agnivade
Copy link
Contributor

@agnivade agnivade commented Aug 29, 2017

Hi,

If anyone is not working on this, may I take this up ?

Also, a question on approaching this - If the appropriate behavior is to return ErrUnexpectedEOF in cases like the ones above, in which cases should we return EOF ?

As far as I understand, EOF is not exactly an error per se. It just signifies that the input has ended. If that is the case, then I believe in all cases where we return EOF, it should be ErrUnexpectedEOF. Let me know what you guys think.

@robpike
Copy link
Contributor

@robpike robpike commented Aug 29, 2017

It's trivial. Line 90 returns EOF, reading from a string. Line 928 converts an EOF from an external scanner (here math/big) into ErrUnexpectedEOF. It's inconsistent, yes. Will it break anything to make that change? I don't know. Does it matter? Not really.

It's really not worth fixing. The Scan part of fmt is poorly designed and I would prefer to leave it alone. It's not a good idea to use it. Scan[f] is an idea borrowed from C that's really not a good fit for the Go library.

Perhaps the documentation should be more discouraging.

@rsc
Copy link
Contributor

@rsc rsc commented Aug 29, 2017

I still intend to look at this to see if there's a clear fix. The problem is the deciding, not the code.

@agnivade
Copy link
Contributor

@agnivade agnivade commented Aug 29, 2017

Sure, let me know what you guys decide. Would be happy to send a CL and reap some commit karma 😝

@rsc rsc modified the milestones: Go1.10, Go1.11 Nov 22, 2017
@gopherbot gopherbot modified the milestones: Go1.11, Unplanned May 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
You can’t perform that action at this time.