On 31/10/2024 23:26, Ping-Ke Shih wrote: >> [...] > Have you run checkpatch before submission? Above two information can be in > formal form as suggestions of checkpatch. Yes, I always do that! Happens that checkpatch sometimes provides good advice (or even point to genuine errors), but sometimes...it's OK to ignore the warnings, specially if we have a reason. Below I detail more about my reasons: > > WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?) > #55: It's like 76 long line, and helped readability on the commit message. In any case, I'll refactor this one in V4, but notice that it'll continue complaining because of point "[0]". That line has 80 or 81 chars, don't recall, but if I reduce it by removing the word "Commit", for example, checkpatch complains I'm writing a commit reference out of preferred format heh So, lose-lose scenario, I can't make the tool fully happy here xD > WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report > #72: This advice would make sense..weren't it for the fact that this Syzkaller issue is already closed heh As I mentioned in the commit message, the main issue with this reproducer is a race in the uevent path vs driver shutdown, addressed by other commit, hence the Syzkaller report is fixed and closed. But...this patch fixes a secondary issue there, and pointing to the Syzkaller issue is useful first to give it credit, since both issues were found by the tool, but also to point to the reproducer, so I kept the Reported tag, but not the Closes one =) > WARNING: The commit message has 'syzkaller', perhaps it also needs a 'Fixes:' tag? > The fixes tag would point to the driver addition, ~10y ago. Stable bots would attempt to backport it for all releases, which I think maybe is not necessary. This is a small issue restricted to (old?) USB devices and emulated devices (in reproducers), so in my commit message instead of adding a Fixes tag, I've added Cc to stable with my suggestion of the versions to backport (6.1.y and 6.6.y basically). That decision is up to you and other maintainers, so feel free to chime in if you prefer to backport to all releases or even *not* backport it at all, I just provided a suggestion based on my understanding about the issue and the relevance of this fix =] Cheers, Guilherme
