Skip to content

Commit

Permalink
fix for off-by-one stack unwinding
Browse files Browse the repository at this point in the history
kubernetes#280 turned println and printf into
wrapper functions, but the underlying *Depth functions kept passing the same 0
depth to l.output and thus any logr.Logger.

That off-by-one error broke JSON tests in Kubernetes, luckily well before any
of the PR went into a release. It would be nice to have regression testing for
this in klog, but we cannot depend on a Logger which does stack unwinding in
the code, so the only thing that could be done is some kind of GitHub action
which combines klog with some external logger.

The depth=1 in println is also not covered by unit tests, but seems to be
correct based on the JSON tests.
  • Loading branch information
pohly committed Feb 10, 2022
1 parent 8d77629 commit 38ac25c
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions klog.go
Expand Up @@ -714,7 +714,7 @@ func (l *loggingT) printlnDepth(s severity, logger *logr.Logger, filter LogFilte
args = filter.Filter(args)
}
fmt.Fprintln(buf, args...)
l.output(s, logger, buf, 0 /* depth */, file, line, false)
l.output(s, logger, buf, depth, file, line, false)
}

func (l *loggingT) print(s severity, logger *logr.Logger, filter LogFilter, args ...interface{}) {
Expand Down Expand Up @@ -758,7 +758,7 @@ func (l *loggingT) printfDepth(s severity, logger *logr.Logger, filter LogFilter
if buf.Bytes()[buf.Len()-1] != '\n' {
buf.WriteByte('\n')
}
l.output(s, logger, buf, 0 /* depth */, file, line, false)
l.output(s, logger, buf, depth, file, line, false)
}

// printWithFileLine behaves like print but uses the provided file and line number. If
Expand Down

0 comments on commit 38ac25c

Please sign in to comment.