linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v1 00/12] Replace strstarts() by str_has_prefix()
@ 2020-12-04 17:03 laniel_francis
  2020-12-04 17:03 ` [RFC PATCH v1 08/12] ide: " laniel_francis
  2020-12-04 17:56 ` [RFC PATCH v1 00/12] " James Bottomley
  0 siblings, 2 replies; 3+ messages in thread
From: laniel_francis @ 2020-12-04 17:03 UTC (permalink / raw)
  To: Russell King, Hauke Mehrtens, Rafał Miłecki,
	Thomas Bogendoerfer, Florian Fainelli, bcm-kernel-feedback-list,
	Herbert Xu, David S. Miller, Ard Biesheuvel, Tomi Valkeinen,
	David Airlie, Daniel Vetter, Laurent Pinchart, Kieran Bingham,
	Alasdair Kergon, Mike Snitzer, dm-devel, Bin Liu,
	Greg Kroah-Hartman, Jessica Yu
  Cc: Francis Laniel, linux-arm-kernel, linux-kernel, linux-mips,
	linux-crypto, linux-efi, dri-devel, linux-renesas-soc, linux-ide,
	linux-usb

From: Francis Laniel <laniel_francis@privacyrequired.com>

Hi.


First, I hope you are fine and the same for your relatives.

In this patch set, I replaced all calls to strstarts() by calls to
str_has_prefix().
Indeed, the kernel has two functions to test if a string begins with an other:
1. strstarts() which returns a bool, so 1 if the string begins with the prefix,
0 otherwise.
2. str_has_prefix() which returns the length of the prefix or 0.

str_has_prefix() was introduced later than strstarts(), in commit 495d714ad140
which also stated that str_has_prefix() should replace strstarts().
This is what this patch set does.

Concerning the patches, the modifications cover different areas of the kernel.
I compiled all of them and they compile smoothly.
Unfortunately, I did not test all of them, so here are the status of the patches
regarding test:
1. Tested with qemu-system-arm using insmod.
2. I do not have access to a bcm47xx MIPS CPU an qemu-system-mips does not
emulate this board.
3. Tested with qemu-system-x86_64 calling
crypto_alloc_skcipher("essiv(authenc(hmac(sha256),cbc(aes)),sha256)", 0, 0)
through LKDTM.
4. Tested with qemu-system-x86_64 using crypsetup.
5. I do not have access to a renesas board and I lack some context to test it
with qemu-system-arm.
6. I do not have access to an OMAP board and I lack some context to test it with
qemu-system-arm.
7. I did not find how to boot from the EFI_STUB with qemu. If you know how to
do, I would be happy to try running this code.
8. I ran qemu-system-x86_64 with a floppy disk attached but impossible to go
through this module code...
9. I do not have access to a bcm63xx MIPS CPU an qemu-system-mips does not
emulate this board.
10. Tested with qemu-system-x86_64 using insmod.
11. I do not have access to an AM335x or DA8xx platforms and I lack some context
to test it with qemu-system-arm.

If you see a way to improve the patches or if I did something wrong with the
mail do not hesitate to ask.


Best regards.

Francis Laniel (12):
  arm: Replace strstarts() by str_has_prefix().
  mips: Replace strstarts() by str_has_prefix().
  crypto: Replace strstarts() by str_has_prefix().
  device-mapper: Replace strstarts() by str_has_prefix().
  renesas: Replace strstarts() by str_has_prefix().
  omap: Replace strstarts() by str_has_prefix().
  efi: Replace strstarts() by str_has_prefix().
  ide: Replace strstarts() by str_has_prefix().
  mips: Replace strstarts() by str_has_prefix().
  module: Replace strstarts() by str_has_prefix().
  musb: Replace strstarts() by str_has_prefix().
  string.h: Remove strstarts().

 arch/arm/kernel/module.c                      | 12 +++++------
 arch/mips/bcm47xx/board.c                     |  4 ++--
 arch/mips/bcm63xx/boards/board_bcm963xx.c     |  2 +-
 crypto/essiv.c                                |  2 +-
 .../firmware/efi/libstub/efi-stub-helper.c    |  2 +-
 drivers/firmware/efi/libstub/gop.c            | 10 +++++-----
 drivers/gpu/drm/omapdrm/dss/base.c            |  2 +-
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |  2 +-
 drivers/ide/ide-floppy.c                      |  4 ++--
 drivers/md/dm-crypt.c                         |  4 ++--
 drivers/usb/musb/musb_cppi41.c                |  4 ++--
 drivers/usb/musb/musb_debugfs.c               | 20 +++++++++----------
 include/linux/string.h                        | 10 ----------
 kernel/module.c                               |  6 +++---
 14 files changed, 37 insertions(+), 47 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [RFC PATCH v1 08/12] ide: Replace strstarts() by str_has_prefix().
  2020-12-04 17:03 [RFC PATCH v1 00/12] Replace strstarts() by str_has_prefix() laniel_francis
@ 2020-12-04 17:03 ` laniel_francis
  2020-12-04 17:56 ` [RFC PATCH v1 00/12] " James Bottomley
  1 sibling, 0 replies; 3+ messages in thread
From: laniel_francis @ 2020-12-04 17:03 UTC (permalink / raw)
  To: David S. Miller; +Cc: Francis Laniel, linux-ide, linux-kernel

From: Francis Laniel <laniel_francis@privacyrequired.com>

The two functions indicates if a string begins with a given prefix.
The only difference is that strstarts() returns a bool while str_has_prefix()
returns the length of the prefix if the string begins with it or 0 otherwise.

Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
---
 drivers/ide/ide-floppy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index f5a2870aaf54..c0b1080e458d 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -495,7 +495,7 @@ static void ide_floppy_setup(ide_drive_t *drive)
 	 * it. It should be fixed as of version 1.9, but to be on the safe side
 	 * we'll leave the limitation below for the 2.2.x tree.
 	 */
-	if (strstarts((char *)&id[ATA_ID_PROD], "IOMEGA ZIP 100 ATAPI")) {
+	if (str_has_prefix((char *)&id[ATA_ID_PROD], "IOMEGA ZIP 100 ATAPI")) {
 		drive->atapi_flags |= IDE_AFLAG_ZIP_DRIVE;
 		/* This value will be visible in the /proc/ide/hdx/settings */
 		drive->pc_delay = IDEFLOPPY_PC_DELAY;
@@ -506,7 +506,7 @@ static void ide_floppy_setup(ide_drive_t *drive)
 	 * Guess what? The IOMEGA Clik! drive also needs the above fix. It makes
 	 * nasty clicking noises without it, so please don't remove this.
 	 */
-	if (strstarts((char *)&id[ATA_ID_PROD], "IOMEGA Clik!")) {
+	if (str_has_prefix((char *)&id[ATA_ID_PROD], "IOMEGA Clik!")) {
 		blk_queue_max_hw_sectors(drive->queue, 64);
 		drive->atapi_flags |= IDE_AFLAG_CLIK_DRIVE;
 		/* IOMEGA Clik! drives do not support lock/unlock commands */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH v1 00/12] Replace strstarts() by str_has_prefix()
  2020-12-04 17:03 [RFC PATCH v1 00/12] Replace strstarts() by str_has_prefix() laniel_francis
  2020-12-04 17:03 ` [RFC PATCH v1 08/12] ide: " laniel_francis
@ 2020-12-04 17:56 ` James Bottomley
  1 sibling, 0 replies; 3+ messages in thread
From: James Bottomley @ 2020-12-04 17:56 UTC (permalink / raw)
  To: laniel_francis, Russell King, Hauke Mehrtens,
	Rafał Miłecki, Thomas Bogendoerfer, Florian Fainelli,
	bcm-kernel-feedback-list, Herbert Xu, David S. Miller,
	Ard Biesheuvel, Tomi Valkeinen, David Airlie, Daniel Vetter,
	Laurent Pinchart, Kieran Bingham, Alasdair Kergon, Mike Snitzer,
	dm-devel, Bin Liu, Greg Kroah-Hartman, Jessica Yu
  Cc: linux-arm-kernel, linux-kernel, linux-mips, linux-crypto,
	linux-efi, dri-devel, linux-renesas-soc, linux-ide, linux-usb

On Fri, 2020-12-04 at 18:03 +0100, laniel_francis@privacyrequired.com
wrote:
> In this patch set, I replaced all calls to strstarts() by calls to
> str_has_prefix(). Indeed, the kernel has two functions to test if a
> string begins with an other:
> 1. strstarts() which returns a bool, so 1 if the string begins with
> the prefix,0 otherwise.
> 2. str_has_prefix() which returns the length of the prefix or 0.
> 
> str_has_prefix() was introduced later than strstarts(), in commit
> 495d714ad140 which also stated that str_has_prefix() should replace
> strstarts(). This is what this patch set does.

What's the reason why?  If you look at the use cases for the
replacement of strstart()  they're all cases where we need to know the
length we're skipping and this is hard coded, leading to potential
errors later.  This is a classic example:  3d739c1f6156 ("tracing: Use
the return of str_has_prefix() to remove open coded numbers").  However
you're not doing this transformation in the conversion, so the
conversion is pretty useless.  I also see no case for replacing
strstart() where we're using it simply as a boolean without needing to
know the length of the prefix.

James



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-12-04 17:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-04 17:03 [RFC PATCH v1 00/12] Replace strstarts() by str_has_prefix() laniel_francis
2020-12-04 17:03 ` [RFC PATCH v1 08/12] ide: " laniel_francis
2020-12-04 17:56 ` [RFC PATCH v1 00/12] " James Bottomley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).