ASoC: SOF: Fix skipping empty trace filters #2516
Conversation
ktrzcinx
force-pushed
the
logger-filtering-fix-n
branch
2 times, most recently
from
44cb561 to
e501c04
Compare
After parsing last entry in trace_filter_parse() function, for line ended with new line character trace_filter_parse_entry() will be called and "\n" will be passed as argument to this function. To skip all possible types of whitechars (which could be introduced during manual filters setup) sscanf() function is used. Space in sscanf() formatter is responsible for whitespaces consumption, %n writes number of characters (whitespaces in this case) read by sscanf(). When input string consist only of whitespaces (read == len), then just skip this content without error notification. Signed-off-by: Karol Trzcinski <[email protected]>
ktrzcinx
force-pushed
the
logger-filtering-fix-n
branch
from
e501c04 to
c0bfcbb
Compare
| return 0; | ||
| sscanf(line, " %n", &read); | ||
| if (read == len) | ||
| return len; | ||
|
|
||
| ret = sscanf(line, " %d %x %d %d %n", &log_level, &uuid_id, &pipe_id, &comp_id, &read); |
There was a problem hiding this comment.
@ktrzcinx Hmm, the sscanf just to read whitespace seems a bit odd. How about if we consume the linefeed in the existing sscanf calll on L51. Would this work?
There was a problem hiding this comment.
sscanf just to read whitespace
rather to check if input string consist only from whitespaces or is empty one, so its eg one of "\0", "\n\0", " \0", " \n\0".
seems a bit odd
I think it shortest and reliable solution as long as reader are familiar with scanf() format string. I didn't use any hack here, it's just standard function usage.
How about if we consume the linefeed in the existing sscanf calll on L51. Would this work?
There already is linefeed consumer in the existing sscanf calll on L51 and it doesn't work in all situations 😄. The case here is parsing last entry of strsep("0 0 0 0;\n", ";"), it will be "\n", so scanning 4 numbers fails for it. Without this fix it works for strsep("0 0 0 0;", ";"), because then last entry is "\0" iirc.
There was a problem hiding this comment.
@ktrzcinx Thanks. I agree this is probably the cleanest, but I'm still not entirely convinced having this sort of relaxed parsing in kernel space is the right place. I mean "0 0 0 0;\n" is an invalid input in the end. Why have extra code in kernel for something user-space can easily just get right.
There was a problem hiding this comment.
Without this change we have some inconsistence - "0 0 0 0;\n" returns parsing error but "0 0 0 0\n" passes, after change it will be consistent.
There was a problem hiding this comment.
I get the point @ktrzcinx about detecting a line with whitespaces only, but I don't get how this change helps you detect
"0 0 0 0;\n" from "0 0 0 0\n"
There was a problem hiding this comment.
@plbossart "0 0 0 0;\n" will be split to "0 0 0 0\0" (parsed with success) and "\n" (rising parse error without this patch). Such and situation doesn't happen for "0 0 0 0\n", because there's no spiting char, and this text will be consumed at once.
|
@xiulipan (you stilll have changes-requested pending)? |
After parsing last entry in trace_filter_parse() function,
for line ended with new line character trace_filter_parse_entry()
will be called and "\n" will be passed as argument to this function.
To skip all possible types of whitechars (which could be introduced
during manual filters setup) sscanf() function is used.
Signed-off-by: Karol Trzcinski [email protected]
Issue: thesofproject/sof#3530