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

Lucas De Marchi lucas.demarchi at intel.com
Wed Jan 19 07:24:47 UTC 2022


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



More information about the Linux-security-module-archive mailing list