On Wed, Jan 26, 2022 at 11:39 AM Lucas De Marchi <lucas.demarchi@xxxxxxxxx> wrote: > > 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@xxxxxxxxx/ > 2) https://lore.kernel.org/all/20210215142137.64476-1-andriy.shevchenko@xxxxxxxxxxxxxxx/#t > > Now there is also the v1 of this same patch series: > https://lore.kernel.org/all/20220119072450.2890107-1-lucas.demarchi@xxxxxxxxx/ > > 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 was 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@xxxxxxxxxxxxxxxxxxxx > 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. However as pointed out in v1 of this patchseries, > onoff(), yesno(), enabledisable(), enableddisabled() have some > issues: the last 2 are hard to read and for the first 2 it would not > be hard to have the symbol to clash with variable names. > From comments in v1, most people were in favor (or at least not > opposed) to using str_on_off(), str_yes_no(), str_enable_disable() > and str_enabled_disabled(). > > 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. One alternative to all of this suggested by Christian König > (43456ba7-c372-84cc-4949-dcb817188e21@xxxxxxx) 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 (or new > renamed version) is easier to remember and use than e.g. %py[DOY] I do not see any impediments to this series to be pulled. Thanks for the work you've done! > Changes in v2: > > - Use str_ prefix and separate other words with underscore: it's a > little bit longer, but should improve readability > > - Patches we re-split due to the rename: first patch adds all the new > functions, then additional patches try to do one conversion at a > time. While doing so, there were some fixes for issues already > present along the way > > - Style suggestions from v1 were adopted > > In v1 it was suggested to apply this in drm-misc. I will leave this to > maintainers to decide: maybe it would be simpler to merge the first > patches on drm-intel-next, wait for the back merge and merge the rest > through drm-misc - my fear is a big conflict with other work going in > drm-intel-next since the bulk of the rename is there. > > I tried to figure out acks and reviews from v1 and apply them to how the > patches are now split. > > thanks > Lucas De Marchi > > Lucas De Marchi (11): > lib/string_helpers: Consolidate string helpers implementation > drm/i915: Fix trailing semicolon > drm/i915: Use str_yes_no() > drm/i915: Use str_enable_disable() > drm/i915: Use str_enabled_disabled() > drm/i915: Use str_on_off() > drm/amd/display: Use str_yes_no() > drm/gem: Sort includes alphabetically > drm: Convert open-coded yes/no strings to yesno() > tomoyo: Use str_yes_no() > cxgb4: Use str_yes_no() > > drivers/gpu/drm/amd/amdgpu/atom.c | 4 +- > .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 14 +- > drivers/gpu/drm/dp/drm_dp.c | 3 +- > drivers/gpu/drm/drm_client_modeset.c | 3 +- > drivers/gpu/drm/drm_gem.c | 23 +- > drivers/gpu/drm/i915/display/g4x_dp.c | 6 +- > .../gpu/drm/i915/display/intel_backlight.c | 3 +- > drivers/gpu/drm/i915/display/intel_ddi.c | 4 +- > drivers/gpu/drm/i915/display/intel_display.c | 46 ++-- > .../drm/i915/display/intel_display_debugfs.c | 74 +++--- > .../drm/i915/display/intel_display_power.c | 4 +- > .../drm/i915/display/intel_display_trace.h | 9 +- > drivers/gpu/drm/i915/display/intel_dp.c | 20 +- > drivers/gpu/drm/i915/display/intel_dpll.c | 3 +- > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 7 +- > drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 7 +- > drivers/gpu/drm/i915/display/intel_fbc.c | 4 +- > drivers/gpu/drm/i915/display/intel_fdi.c | 8 +- > drivers/gpu/drm/i915/display/intel_hdmi.c | 3 +- > drivers/gpu/drm/i915/display/intel_sprite.c | 6 +- > drivers/gpu/drm/i915/display/vlv_dsi_pll.c | 3 +- > .../gpu/drm/i915/gem/selftests/huge_pages.c | 9 +- > .../drm/i915/gem/selftests/i915_gem_context.c | 7 +- > drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 3 +- > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 11 +- > .../drm/i915/gt/intel_execlists_submission.c | 7 +- > drivers/gpu/drm/i915/gt/intel_gt_pm.c | 3 +- > drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 52 ++-- > drivers/gpu/drm/i915/gt/intel_rc6.c | 5 +- > drivers/gpu/drm/i915/gt/intel_reset.c | 3 +- > drivers/gpu/drm/i915/gt/intel_rps.c | 13 +- > drivers/gpu/drm/i915/gt/intel_sseu.c | 9 +- > drivers/gpu/drm/i915/gt/intel_sseu_debugfs.c | 10 +- > drivers/gpu/drm/i915/gt/selftest_timeline.c | 3 +- > drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 5 +- > drivers/gpu/drm/i915/gt/uc/intel_guc_log.c | 5 +- > drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c | 6 +- > drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 4 +- > drivers/gpu/drm/i915/gt/uc/intel_uc.c | 14 +- > drivers/gpu/drm/i915/gt/uc/intel_uc_debugfs.c | 20 +- > drivers/gpu/drm/i915/i915_debugfs.c | 17 +- > drivers/gpu/drm/i915/i915_driver.c | 4 +- > drivers/gpu/drm/i915/i915_gpu_error.c | 9 +- > drivers/gpu/drm/i915/i915_params.c | 5 +- > drivers/gpu/drm/i915/i915_utils.h | 21 +- > drivers/gpu/drm/i915/intel_device_info.c | 8 +- > drivers/gpu/drm/i915/intel_dram.c | 10 +- > drivers/gpu/drm/i915/intel_pm.c | 14 +- > drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c | 4 +- > drivers/gpu/drm/i915/selftests/i915_active.c | 3 +- > drivers/gpu/drm/i915/vlv_suspend.c | 3 +- > drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c | 5 +- > drivers/gpu/drm/radeon/atom.c | 3 +- > drivers/gpu/drm/v3d/v3d_debugfs.c | 11 +- > drivers/gpu/drm/virtio/virtgpu_debugfs.c | 4 +- > .../ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 249 ++++++++++-------- > include/linux/string_helpers.h | 20 ++ > security/tomoyo/audit.c | 2 +- > security/tomoyo/common.c | 19 +- > security/tomoyo/common.h | 1 - > 60 files changed, 482 insertions(+), 373 deletions(-) > > -- > 2.34.1 > -- With Best Regards, Andy Shevchenko
