| Message ID | 20240726071416.36759-1-xni@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mdadm: fix coverity issues | expand |
Dear Xiao, Thank you for taking care of these things. Some comments on minor things. Am 26.07.24 um 09:14 schrieb Xiao Ni: […] > Xiao Ni (14): > mdadm/Grow: fix coverity issue CHECKED_RETURN > mdadm/Grow: fix coverity issue RESOURCE_LEAK > mdadm/Grow: fix coverity issue STRING_OVERFLOW > mdadm/Incremental: fix coverity issues. I’d remove the dot/period at the end > mdadm/mdmon: fix coverity issue CHECKED_RETURN > mdadm/mdmon: fix coverity issue RESOURCE_LEAK > mdadm/mdopen: fix coverity issue CHECKED_RETURN > mdadm/mdopen: fix coverity issue STRING_OVERFLOW > mdadm/mdstat: fix coverity issue CHECKED_RETURN > mdadm/super0: fix coverity issue CHECKED_RETURN and EVALUATION_ORDER > mdadm/super1: fix coverity issue CHECKED_RETURN > mdadm/super1: fix coverity issue DEADCODE > mdadm/super1: fix coverity issue EVALUATION_ORDER > mdadm/super1: fix coverity issue RESOURCE_LEAK In my opinion, naming the tool reporting the issue in the commit message summary is not beneficial, and I’d prefer to have more detail on the change in there. The tool could be named/credited in the commit message body. Kind regards, Paul
On Fri, 26 Jul 2024 10:01:29 +0200 Paul Menzel <pmenzel@molgen.mpg.de> wrote: > In my opinion, naming the tool reporting the issue in the commit message > summary is not beneficial, and I’d prefer to have more detail on the > change in there. The tool could be named/credited in the commit message > body. Hmm, I didn't put a huge patience into that, I accepted similar commits from Nigel on GH, like this one: https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/commit/?id=1b4b73fd535a6487075e98f620454ff2e13b5240 However, he used the exact problem description reported by the tool in commit message. This is not exactly the same style. I may looks like hypocrite, asking for changes from Xiao now. For that reason I'm fine with current style but I doesn't mean that I disagree with Paul. I agree with Paul. Xiao, let me know if you would like to rework descriptions or you would prefer me to pick it in this form. Mariusz
On Fri, 26 Jul 2024 15:14:02 +0800 Xiao Ni <xni@redhat.com> wrote: > V2: replace close with close_fd and use is_fd_valid > V3: Fix errors reported by checkpatch > V4: > don't need check fd is valid before close_fd > replace strcat with snprintf > replace dev_name with dev_path_name > > Xiao Ni (14): > mdadm/Grow: fix coverity issue CHECKED_RETURN > mdadm/Grow: fix coverity issue RESOURCE_LEAK > mdadm/Grow: fix coverity issue STRING_OVERFLOW > mdadm/Incremental: fix coverity issues. > mdadm/mdmon: fix coverity issue CHECKED_RETURN > mdadm/mdmon: fix coverity issue RESOURCE_LEAK > mdadm/mdopen: fix coverity issue CHECKED_RETURN > mdadm/mdopen: fix coverity issue STRING_OVERFLOW > mdadm/mdstat: fix coverity issue CHECKED_RETURN > mdadm/super0: fix coverity issue CHECKED_RETURN and EVALUATION_ORDER > mdadm/super1: fix coverity issue CHECKED_RETURN > mdadm/super1: fix coverity issue DEADCODE > mdadm/super1: fix coverity issue EVALUATION_ORDER > mdadm/super1: fix coverity issue RESOURCE_LEAK > > Grow.c | 87 ++++++++++++++++++++++++++++++++++++++++----------- > Incremental.c | 20 ++++++------ > mdmon.c | 20 +++++++++--- > mdopen.c | 8 +++-- > mdstat.c | 12 +++++-- > super0.c | 10 ++++-- > super1.c | 32 ++++++++++++++----- > 7 files changed, 139 insertions(+), 50 deletions(-) > Applied! Sorry for the delay. I was few days out of office! Mariusz