-
Notifications
You must be signed in to change notification settings - Fork 424
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
fix empty arguments resolution #4442
base: main
Are you sure you want to change the base?
Conversation
Let's change all parsers inside this file that currently return an empty value on an error to return the raw value instead |
Okay, but I think the code is still going to look weird because if we just print the information without any need for processing it behind the scenes, why would we write the function like this: func funcArg(arg *trace.Argument, val uint64) {
arg.Type = "string"
argument, err := parsers.funcArg(mode)
if err != nil {
arg.Value = ""
return
}
arg.Value = argument.String()
} instead of like this: func funcArg(arg *tracee.Argument, val uint64) {
arg.Type = "string"
arg.Value = val
} or just remove it entirely? It looks pretty weird that we check for an error and don't even use it. Note: Not every function behaves like this, but most of them do. |
We do use the error, that way we know to use the raw value. |
@@ -46,7 +46,7 @@ func parseBPFProgType(arg *trace.Argument, progType uint64) { | |||
arg.Type = "string" | |||
bpfProgTypeArgument, err := parsers.ParseBPFProgType(progType) | |||
if err != nil { | |||
arg.Value = "" | |||
arg.Value = progType |
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.
If the raw value is used, the value type is inconsistent. We should either change the type accordingly or format the raw value as a string (preferrable in my opinion).
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 agree - better to format the raw argument as a string
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.
Yes, that sounds good. However, as discussed earlier, raw values are used when Tracee can't identify the data. In this code, forming the raw value isn't the best practice because that happens in the pkg/parsers module.
@@ -157,7 +157,7 @@ func parseBPFCmd(arg *trace.Argument, cmd uint64) { | |||
arg.Type = "string" | |||
bpfCommandArgument, err := parsers.ParseBPFCmd(cmd) | |||
if err != nil { | |||
arg.Value = "" | |||
arg.Value = cmd |
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.
Same as above
Because the parsers return the parsed value of the argument. For example, if some (fake) argument has a raw value 0x4 which means F_WRITE, the output string will be "F_WRITE". However, if it has a value of 0x20, but the parsers don't know this value, an error will be returned. In this case, we can simply return the string value of the raw argument, which is "0x20" |
1. Explain what the PR does
"Replace me with
make check-pr
output"2. Explain how to test it
change empty sting to hex value
3. Other comments
Resolves #3892