Messages in this thread | | | Date | Sun, 29 Aug 2021 18:14:55 +0200 | From | Len Baker <> | Subject | Re: [PATCH v4] EDAC/mc: Prefer strscpy over strcpy |
| |
Hi,
On Fri, Aug 27, 2021 at 07:54:07PM +0200, Borislav Petkov wrote: > On Fri, Aug 27, 2021 at 07:36:33PM +0200, Len Baker wrote: > > Well, the main purpose is to clean up the proliferation of str*cpy functions. > > One task is to remove the strcpy uses: The first step (previous step) would > > be to remove all the strcpy uses. Then, as a second step remove all the > > strcpy implementations. > > > > I hope that this clarify your question. > > Yes, it does. > > Now lemme clarify why I'm asking: when your patch is committed to the > kernel tree and someone reads its commit message months or even years > from now - and those who do that are mostly maintainers trying to figure > out why stuff was done the way it was - they will read: > > "This is a previous step in the path to remove the strcpy() function > entirely from the kernel." > > and wonder what previous step that is what the following step is... > > So, long story short, your commit message should be complete on its own > and understandable without any references to things which might not be > as clear and self-evident in the future as they are now. > > Makes sense?
Ok, understood. Thanks for the advise and guidance.
> > Also, if you're wondering if you should send the patch with the error > checking of strscpy() added, as I requested, even if it might look > superfluous now, yes you should. > > Even if it looks impossible now, we might change some of those defines > in the future and forget to touch the logic which generates e->label and > we might end up exhausting that string. > > So it would be a lot more robust if something would catch that change, > albeit seemingly redundant now. > > I sincerely hope that clears up things.
Yes, it clears up things. However I think the same that Joe:
From Joe Perches:
[...] I still think scnprintf is _way_ more common and intelligible as a construct than this odd strscpy with required error checking. [...]
So, I will send a new version for review with the commit message updated and using the scnprintf. This way we can discuss using a real patch. Anyway thanks for the review.
Regards, Len
| |