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

crypto/tls: small inconsistency around error formatting on crypto/generate_cert #34848

Closed
fenos opened this issue Oct 11, 2019 · 4 comments
Closed

Comments

@fenos
Copy link
Contributor

@fenos fenos commented Oct 11, 2019

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

$ go version go1.12.5 darwin/amd64

Does this issue reproduce with the latest release?

Yes, the inconsistency lives on the master branch

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/fenos/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/fenos/Documents/code/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.12.5/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.12.5/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/hd/7fyxcyhn01x2pmdqrs892v4h0000gp/T/go-build120066066=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Going trough the source code and i've noticed small inconsistencies in the code around the error formatting.

What did you expect to see?

  • All errors are consistently using %s for interpolating errors
  • Have the error included in every fatal error report

What did you see instead?

@fenos

This comment has been minimized.

Copy link
Contributor Author

@fenos fenos commented Oct 11, 2019

I intend to work on a fix for this

@ianlancetaylor ianlancetaylor changed the title Small inconsistency around error formatting on crypto/generate_cert crypto/tls: small inconsistency around error formatting on crypto/generate_cert Oct 11, 2019
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 11, 2019

Thanks for the report. It's fine to use %v to print a value of type error. If anything %v is more common than %s.

The log.Fatalf line should probably just be log.Fatal.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Oct 11, 2019

Change https://golang.org/cl/200679 mentions this issue: crypto/tls: Fixes #34848

@fenos

This comment has been minimized.

Copy link
Contributor Author

@fenos fenos commented Oct 11, 2019

@iamoryanmoshe yes absolutely, i'm just trying to make things consistent across the file.
Would you be happy to change all the %s to %v?

On the second bit, I think it would still be good to have the error printed out within the message

@gopherbot gopherbot closed this in 2ac8f79 Oct 11, 2019
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
3 participants
You can’t perform that action at this time.