-
Notifications
You must be signed in to change notification settings - Fork 595
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
follow-up to #3343: review uses of bufio.Scanner #3370
Comments
Note: For a cursory review:
I believe that:
That leaves 7, which is certainly a problem Hey @CerberusQc May I interest you in patching https://github.com/containerd/nerdctl/blob/main/pkg/composer/pipetagger/pipetagger.go#L97-L110 ? :-) |
Sorry for the late response, I am currently moving into a new house, I won't be able to work on this before 1-2 weeks. I am wondering if it would be better to move the function into it's own struct to be more generic and allow to stay DRY in that case. If so, I am not really familiar with GO folders structure, where should I put the generic class ? I was thinking about something like type LogProcessor struct {
wg *sync.WaitGroup
reader io.Reader
dataChan chan string
}
func NewLogProcessor(wg *sync.WaitGroup, reader io.Reader, dataChan chan string) *LogProcessor {
return &LogProcessor{
wg: wg,
reader: reader,
dataChan: dataChan,
}
}
func (p *LogProcessor) Process() {
defer p.wg.Done()
defer close(p.dataChan)
r := bufio.NewReader(p.reader)
var err error
for err == nil {
var s string
s, err = r.ReadString('\n')
if len(s) > 0 {
p.dataChan <- strings.TrimSuffix(s, "\n")
}
if err != nil && err != io.EOF {
log.L.WithError(err).Error("failed to read log")
}
}
} This is just a draft and did not run it, naming need improvement. |
No problem. Congrats on the new house! On your code suggestion - let's chat on your (future) PR when you get there. See you in a few weeks then ;). |
What is the problem you're trying to solve
As discussed in #3343, the usage of bufio.Scanner when we do not have the control over the length of the message processed by the scanner can put the Scanner in a I/O error state failing silently and blocking the process sending a message.
@apostasie recommend that the usage of bufio.Scanner across the project is revisited.
Describe the solution you'd like
A solution is suggested with #3366 but might not be a "one size fit all solution".
Additional context
Documentation
The documentation state the following:
Scanning stops unrecoverably at EOF, the first I/O error, or a token too large to fit in the Scanner.Buffer. When a scan stops, the reader may have advanced arbitrarily far past the last token. Programs that need more control over error handling or large tokens, or must run sequential scans on a reader, should use bufio.Reader instead.
The text was updated successfully, but these errors were encountered: