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.

Sign up for GitHub

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

Jump to bottom

log: %s semantics different from stdlib for nil pointers #64158

Closed
andreimatei opened this issue Apr 23, 2021 · 2 comments · Fixed by #67064
Closed

log: %s semantics different from stdlib for nil pointers #64158

andreimatei opened this issue Apr 23, 2021 · 2 comments · Fixed by #67064
Assignees
@knz
Labels
A-logging In and around the logging infrastructure. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-server-and-security DB Server & Security

Comments

@andreimatei
Copy link
Contributor

I've seen this in a log message, resulting from using %s for printing a nil pointer of a type that implements SafeFormatter on a value receiver.

%!s(PANIC=SafeFormatter method: value method github.com/cockroachdb/cockroach/pkg/roachpb.Lease.SafeFormat called using nil *Lease pointer

This is different from what fmt.Printf does for the Stringer interface, where it prints <nil>. If possible, it'd certainly take the <nil>. I've found this thread that might describe how to handle this case in the logging lib: golang/go#20995
@andreimatei andreimatei added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-logging In and around the logging infrastructure. T-server-and-security DB Server & Security labels Apr 23, 2021
@knz
Copy link
Contributor

knz commented Apr 23, 2021

Can you file this upstream in the redact repo thanks

@andreimatei
Copy link
Contributor Author

done cockroachdb/redact#15

@exalate-issue-sync exalate-issue-sync bot removed the T-server-and-security DB Server & Security label May 29, 2021
@jlinder jlinder added the T-server-and-security DB Server & Security label May 29, 2021
This was referenced Jun 25, 2021
Merged
Merged
@craig craig bot closed this as completed in c452a0c Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@knz knz

Labels
A-logging In and around the logging infrastructure. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-server-and-security DB Server & Security
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

deps: bump cockroachdb/redact knz/cockroach
3 participants
@jlinder @andreimatei @knz