[PATCH 0/3] lib/string_helpers: Add a few string helpers

* [PATCH 0/3] lib/string_helpers: Add a few string helpers
@ 2022-01-19  7:24 ` Lucas De Marchi
  0 siblings, 0 replies; 160+ messages in thread
From: Lucas De Marchi @ 2022-01-19  7:24 UTC (permalink / raw)
  To: linux-kernel, intel-gfx, dri-devel, amd-gfx,
	linux-security-module, nouveau, netdev
  Cc: Alex Deucher, Andrew Morton, Andy Shevchenko, Andy Shevchenko,
	Ben Skeggs, Christian König, Chris Wilson, Daniel Vetter,
	David Airlie, David S . Miller, Emma Anholt, Eryk Brol,
	Francis Laniel, Greg Kroah-Hartman, Harry Wentland,
	Jakub Kicinski, Jani Nikula, Joonas Lahtinen, Julia Lawall,
	Kentaro Takeda, Leo Li, Mikita Lipski, Petr Mladek,
	Rahul Lakkireddy, Raju Rangoju, Rasmus Villemoes, Rodrigo Vivi,
	Sakari Ailus, Sergey Senozhatsky, Steven Rostedt,
	Vishal Kulkarni

Add some helpers under lib/string_helpers.h so they can be used
throughout the kernel. When I started doing this there were 2 other
previous attempts I know of, not counting the iterations each of them
had:

1) https://lore.kernel.org/all/20191023131308.9420-1-jani.nikula@intel.com/
2) https://lore.kernel.org/all/20210215142137.64476-1-andriy.shevchenko@linux.intel.com/#t

Going through the comments I tried to find some common ground and
justification for what is in here, addressing some of the concerns
raised.

a. This version should be a drop-in replacement for what is currently in
   the tree, with no change in behavior or binary size. For binary
   size what I checked wat that the linked objects in the end have the
   same size (gcc 11). From comments in the previous attempts this seems
   also the case for earlier compiler versions

b. I didn't change the function name to choice_* as suggested by Andrew
   Morton in 20191023155619.43e0013f0c8c673a5c508c1e@linux-foundation.org
   because other people argumented in favor of shorter names for these
   simple helpers - if they are long and people simply not use due to
   that, we failed

c. Use string_helper.h for these helpers - pulling string.h in the
   compilations units was one of the concerns and I think re-using this
   already existing header is better than creating a new string-choice.h

d. This doesn't bring onoff() helper as there are some places in the
   kernel with onoff as variable - another name is probably needed for
   this function in order not to shadow the variable, or those variables
   could be renamed.  Or if people wanting  <someprefix>
   try to find a short one

e. One alternative to all of this suggested by Christian König
   (43456ba7-c372-84cc-4949-dcb817188e21@amd.com) would be to add a
   printk format. But besides the comment, he also seemed to like
   the common function. This brought the argument from others that the
   simple yesno()/enabledisable() already used in the code is easier to
   remember and use than e.g. %py[DOY]

Last patch also has some additional conversion of open coded cases. I
preferred starting with drm/ since this is "closer to home".

I hope this is a good summary of the previous attempts and a way we can
move forward.

Andrew Morton, Petr Mladek, Andy Shevchenko: if this is accepted, my
proposal is to take first 2 patches either through mm tree or maybe
vsprintf. Last patch can be taken later through drm.

thanks
Lucas De Marchi

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: David Airlie <airlied@linux.ie>
Cc: David S. Miller <davem@davemloft.net>
Cc: Emma Anholt <emma@anholt.net>
Cc: Eryk Brol <eryk.brol@amd.com>
Cc: Francis Laniel <laniel_francis@privacyrequired.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Julia Lawall <julia.lawall@lip6.fr>
Cc: Kentaro Takeda <takedakn@nttdata.co.jp>
Cc: Leo Li <sunpeng.li@amd.com>
Cc: Mikita Lipski <mikita.lipski@amd.com>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
Cc: Raju Rangoju <rajur@chelsio.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Vishal Kulkarni <vishal@chelsio.com>

Lucas De Marchi (3):
  lib/string_helpers: Consolidate yesno() implementation
  lib/string_helpers: Add helpers for enable[d]/disable[d]
  drm: Convert open yes/no strings to yesno()

 drivers/gpu/drm/amd/amdgpu/atom.c              |  3 ++-
 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c  |  6 +-----
 drivers/gpu/drm/drm_client_modeset.c           |  3 ++-
 drivers/gpu/drm/drm_dp_helper.c                |  3 ++-
 drivers/gpu/drm/drm_gem.c                      |  3 ++-
 drivers/gpu/drm/i915/i915_utils.h              | 15 ---------------
 drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c  |  4 +++-
 drivers/gpu/drm/radeon/atom.c                  |  3 ++-
 drivers/gpu/drm/v3d/v3d_debugfs.c              | 11 ++++++-----
 drivers/gpu/drm/virtio/virtgpu_debugfs.c       |  3 ++-
 .../net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 11 -----------
 include/linux/string_helpers.h                 |  4 ++++
 security/tomoyo/audit.c                        |  2 +-
 security/tomoyo/common.c                       | 18 ++++--------------
 security/tomoyo/common.h                       |  1 -
 15 files changed, 31 insertions(+), 59 deletions(-)

-- 
2.34.1

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