Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The patch implementation directly contradicts the stated intent in the commit message regarding which log lines are dropped from the 3rd stanza. --
commit 508361dc136fe2a26caa1e30ec9b645bd4c49e81 Author: Jim Cromie <[email protected]> dyndbg: reduce verbose/debug clutter This patch aims to reduce excessive logging in dynamic debug, specifically shrinking and dropping several verbose messages during query processing and module addition. > - shrinks 3 lines of 2nd stanza to single line > - drops 1st 2 lines of 3rd stanza > 3rd line is like 1st, with result, not procedure. > 2nd line is just status, retold in 4th, with more info. > > New output: > > dyndbg: query 0: "class DRM_UT_CORE +p" > dyndbg: split into words: "class" "DRM_UT_CORE" "+p" > dyndbg: op='+' flags=0x1 maskp=0xffffffff > dyndbg: processed 1 queries, with 0 matches, 0 errs [Severity: Medium] Does this commit message accurately reflect the changes made in the code? The commit message states the intent is to drop the first two lines of the 3rd stanza (the "parsed" and "no matches" logs) while keeping the third line ("no-match"/"applied") since it shows the result rather than the procedure. However, looking at the code changes below, the implementation drops the second and third lines but appears to keep the first line (the "parsed" log printed from ddebug_parse_query() right before ddebug_change() is called). This seems to result in keeping the procedure log and dropping the result log, reversing the stated intent. Additionally, should the "New output" example in the commit message be updated to include the "parsed" log line that is still printed? > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c > index 6d9dbeb68ae83..dfed3725aa44b 100644 > --- a/lib/dynamic_debug.c > +++ b/lib/dynamic_debug.c > @@ -277,9 +277,6 @@ static int ddebug_change(const struct ddebug_query *query, > } > mutex_unlock(&ddebug_lock); > > - if (!nfound && verbose) > - pr_info("no matches for query\n"); > - > return nfound; > } [ ... ] > @@ -570,10 +565,7 @@ static int ddebug_exec_query(char *query_string, const > char *modname) > return -EINVAL; > } > /* actually go and implement the change */ > - nfound = ddebug_change(&query, &modifiers); > - vpr_info_dq(&query, nfound ? "applied" : "no-match"); > - > - return nfound; > + return ddebug_change(&query, &modifiers); > } > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=13
