| Message ID | 20250226-misc-const-v2-0-ab655a4a29cc@posteo.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | usb: storage: Mark various arrays as const | expand |
On Wed, Feb 26, 2025 at 03:17:22PM +0100, 'Jonathan Neuschäfer via B4 Relay' via USB Mass Storage on Linux wrote: > While reading code, I noticed that some arrays in USB mass storage > drivers are declared static but not const, even though they are not > modified. This patchset marks them const. > > All patches were compile-tested. > > Signed-off-by: Jonathan Neuschäfer <j.ne@posteo.net> > --- > Changes in v2: > - Add new patches 2-9 > - Use consistent authorship information > - Link to v1: https://lore.kernel.org/r/20250225-misc-const-v1-1-121ff3b86437@posteo.net The patches themselves look good, but I still think you should explain in the patch descriptions why declaring these arrays const is worth doing. Merely saying _what_ you are doing isn't good enough. We can tell what a patch does just by reading it. What we can't always tell is _why_ you would want to do it. That is what needs to be explained. The explanation doesn't have to be terribly long or detailed, but you should not omit it entirely. Alan Stern
On Wed, Feb 26, 2025 at 12:02:02PM -0500, Alan Stern wrote: > On Wed, Feb 26, 2025 at 03:17:22PM +0100, 'Jonathan Neuschäfer via B4 Relay' via USB Mass Storage on Linux wrote: > > While reading code, I noticed that some arrays in USB mass storage > > drivers are declared static but not const, even though they are not > > modified. This patchset marks them const. > > > > All patches were compile-tested. > > > > Signed-off-by: Jonathan Neuschäfer <j.ne@posteo.net> > > --- > > Changes in v2: > > - Add new patches 2-9 > > - Use consistent authorship information > > - Link to v1: https://lore.kernel.org/r/20250225-misc-const-v1-1-121ff3b86437@posteo.net > > The patches themselves look good, but I still think you should explain > in the patch descriptions why declaring these arrays const is worth > doing. > > Merely saying _what_ you are doing isn't good enough. We can tell what > a patch does just by reading it. What we can't always tell is _why_ you > would want to do it. That is what needs to be explained. > > The explanation doesn't have to be terribly long or detailed, but you > should not omit it entirely. Fair enough, I'll add such explanations to the patches. Roughly, my motivations are: - Moving data to read-only memory can prevent unintended modifications and the hard-to-debug issue that might follow - Const makes it easier for human readers to know what to expect Best regards
While reading code, I noticed that some arrays in USB mass storage drivers are declared static but not const, even though they are not modified. This patchset marks them const. All patches were compile-tested. Signed-off-by: Jonathan Neuschäfer <j.ne@posteo.net> --- Changes in v2: - Add new patches 2-9 - Use consistent authorship information - Link to v1: https://lore.kernel.org/r/20250225-misc-const-v1-1-121ff3b86437@posteo.net --- Jonathan Neuschäfer (9): usb: storage: jumpshot: Use const for constant arrays usb: storage: transport: Use const for constant array usb: storage: alauda: Use const for card ID array usb: storage: datafab: Use const for constant arrays usb: storage: initializers: Use const for constant array usb: storage: realtek_cr: Use const for constant arrays usb: storage: sddr09: Use const for constant arrays usb: storage: sddr55: Use const for constant arrays usb: storage: shuttle_usbat: Use const for constant array drivers/usb/storage/alauda.c | 8 ++++---- drivers/usb/storage/datafab.c | 14 +++++++------- drivers/usb/storage/initializers.c | 2 +- drivers/usb/storage/jumpshot.c | 10 +++++----- drivers/usb/storage/realtek_cr.c | 6 +++--- drivers/usb/storage/sddr09.c | 14 +++++++------- drivers/usb/storage/sddr55.c | 4 ++-- drivers/usb/storage/shuttle_usbat.c | 2 +- drivers/usb/storage/transport.c | 2 +- 9 files changed, 31 insertions(+), 31 deletions(-) --- base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b change-id: 20240401-misc-const-e7b4cf20d5f9 Best regards,