-
Notifications
You must be signed in to change notification settings - Fork 6
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
proc: Fix parsing Kernel Module lines where refcount is "-" #26
Conversation
8a06cd8
to
6faf716
Compare
77e881a
to
92a915f
Compare
proc/proc.go
Outdated
func parseKernelModules(content []byte) ([]kernelModule, error) { | ||
var ( | ||
modules []kernelModule | ||
scanner = bufio.NewScanner(bytes.NewReader(content)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why slurp all the content and then construct a scanner on top of it -- why not construct a scanner directly on top of the file like we did in the old code?
Doesn't really matter since these files should be small but it seems semantically unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense
proc/proc.go
Outdated
return &symmap, nil | ||
type kernelModule struct { | ||
name string | ||
size int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a slight preference to being explicit about sizes (e.g. int64 if this can indeed be bigger than 2 GiB)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only went with int because the rest of the codebase used ints where this ends up being used, but let me see how much work it would be to change this.
proc/proc.go
Outdated
if size > int64(math.MinInt32) && size < int64(math.MaxInt32) { | ||
return int(size), nil | ||
} | ||
|
||
return 0, fmt.Errorf("size value out of range: %q", sizeStr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the purpose of this check? If you want to enforce that the value is within a 32-bit range, why not call parseInt
with bitSize = 32
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was to please CodeQL which was complaining about not guarding this.
bce0ba7
to
b1f1672
Compare
No description provided.