All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@intel.com>
To: linux-kernel@vger.kernel.org
Cc: Jani Nikula <jani.nikula@intel.com>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	intel-gfx@lists.freedesktop.org,
	Vishal Kulkarni <vishal@chelsio.com>,
	netdev@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Julia Lawall <julia.lawall@lip6.fr>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>
Subject: [PATCH v4] string-choice: add yesno(), onoff(), enableddisabled(), plural() helpers
Date: Wed, 23 Oct 2019 16:13:08 +0300	[thread overview]
Message-ID: <20191023131308.9420-1-jani.nikula@intel.com> (raw)

The kernel has plenty of ternary operators to choose between constant
strings, such as condition ? "yes" : "no", as well as value == 1 ? "" :
"s":

$ git grep '? "yes" : "no"' | wc -l
258
$ git grep '? "on" : "off"' | wc -l
204
$ git grep '? "enabled" : "disabled"' | wc -l
196
$ git grep '? "" : "s"' | wc -l
25

Additionally, there are some occurences of the same in reverse order,
split to multiple lines, or otherwise not caught by the simple grep.

Add helpers to return the constant strings. Remove existing equivalent
and conflicting functions in i915, cxgb4, and USB core. Further
conversion can be done incrementally.

The main goal here is to abstract recurring patterns, and slightly clean
up the code base by not open coding the ternary operators.

Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: Vishal Kulkarni <vishal@chelsio.com>
Cc: netdev@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

v2: add string-choice.[ch] to not clutter kernel.h and to actually save
space on string constants.

v3: back to static inlines based on Rasmus' feedback

v4: Massaged commit message about space savings to make it less fluffy
based on Rasmus' feedback.

Example of further cleanup possibilities are at [1], to be done
incrementally afterwards.

[1] http://lore.kernel.org/r/20190903133731.2094-2-jani.nikula@intel.com
---
 drivers/gpu/drm/i915/i915_utils.h             | 16 +---------
 .../ethernet/chelsio/cxgb4/cxgb4_debugfs.c    | 12 +------
 drivers/usb/core/config.c                     |  6 +---
 drivers/usb/core/generic.c                    |  6 +---
 include/linux/string-choice.h                 | 31 +++++++++++++++++++
 5 files changed, 35 insertions(+), 36 deletions(-)
 create mode 100644 include/linux/string-choice.h

diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index 562f756da421..794f02a90efe 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -28,6 +28,7 @@
 #include <linux/list.h>
 #include <linux/overflow.h>
 #include <linux/sched.h>
+#include <linux/string-choice.h>
 #include <linux/types.h>
 #include <linux/workqueue.h>
 
@@ -395,21 +396,6 @@ wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms)
 #define MBps(x) KBps(1000 * (x))
 #define GBps(x) ((u64)1000 * MBps((x)))
 
-static inline const char *yesno(bool v)
-{
-	return v ? "yes" : "no";
-}
-
-static inline const char *onoff(bool v)
-{
-	return v ? "on" : "off";
-}
-
-static inline const char *enableddisabled(bool v)
-{
-	return v ? "enabled" : "disabled";
-}
-
 static inline void add_taint_for_CI(unsigned int taint)
 {
 	/*
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
index ae6a47dd7dc9..d9123dae1d00 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
@@ -35,6 +35,7 @@
 #include <linux/seq_file.h>
 #include <linux/debugfs.h>
 #include <linux/string_helpers.h>
+#include <linux/string-choice.h>
 #include <linux/sort.h>
 #include <linux/ctype.h>
 
@@ -2023,17 +2024,6 @@ static const struct file_operations rss_debugfs_fops = {
 /* RSS Configuration.
  */
 
-/* Small utility function to return the strings "yes" or "no" if the supplied
- * argument is non-zero.
- */
-static const char *yesno(int x)
-{
-	static const char *yes = "yes";
-	static const char *no = "no";
-
-	return x ? yes : no;
-}
-
 static int rss_config_show(struct seq_file *seq, void *v)
 {
 	struct adapter *adapter = seq->private;
diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index 151a74a54386..52cee9067eb4 100644
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -10,6 +10,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/device.h>
+#include <linux/string-choice.h>
 #include <asm/byteorder.h>
 #include "usb.h"
 
@@ -19,11 +20,6 @@
 #define USB_MAXCONFIG			8	/* Arbitrary limit */
 
 
-static inline const char *plural(int n)
-{
-	return (n == 1 ? "" : "s");
-}
-
 static int find_next_descriptor(unsigned char *buffer, int size,
     int dt1, int dt2, int *num_skipped)
 {
diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c
index 38f8b3e31762..a784a09794d6 100644
--- a/drivers/usb/core/generic.c
+++ b/drivers/usb/core/generic.c
@@ -21,14 +21,10 @@
 
 #include <linux/usb.h>
 #include <linux/usb/hcd.h>
+#include <linux/string-choice.h>
 #include <uapi/linux/usb/audio.h>
 #include "usb.h"
 
-static inline const char *plural(int n)
-{
-	return (n == 1 ? "" : "s");
-}
-
 static int is_rndis(struct usb_interface_descriptor *desc)
 {
 	return desc->bInterfaceClass == USB_CLASS_COMM
diff --git a/include/linux/string-choice.h b/include/linux/string-choice.h
new file mode 100644
index 000000000000..320b598bd8f0
--- /dev/null
+++ b/include/linux/string-choice.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef __STRING_CHOICE_H__
+#define __STRING_CHOICE_H__
+
+#include <linux/types.h>
+
+static inline const char *yesno(bool v)
+{
+	return v ? "yes" : "no";
+}
+
+static inline const char *onoff(bool v)
+{
+	return v ? "on" : "off";
+}
+
+static inline const char *enableddisabled(bool v)
+{
+	return v ? "enabled" : "disabled";
+}
+
+static inline const char *plural(long v)
+{
+	return v == 1 ? "" : "s";
+}
+
+#endif /* __STRING_CHOICE_H__ */
-- 
2.20.1


WARNING: multiple messages have this Message-ID (diff)
From: Jani Nikula <jani.nikula@intel.com>
To: linux-kernel@vger.kernel.org
Cc: linux-usb@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jani Nikula <jani.nikula@intel.com>,
	netdev@vger.kernel.org, intel-gfx@lists.freedesktop.org,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Julia Lawall <julia.lawall@lip6.fr>,
	Vishal Kulkarni <vishal@chelsio.com>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: [Intel-gfx] [PATCH v4] string-choice: add yesno(), onoff(), enableddisabled(), plural() helpers
Date: Wed, 23 Oct 2019 16:13:08 +0300	[thread overview]
Message-ID: <20191023131308.9420-1-jani.nikula@intel.com> (raw)
Message-ID: <20191023131308.9sE9upQuZxojDZ0aGEOzsibNjtu9sKLMyVOlBtvkzPs@z> (raw)

The kernel has plenty of ternary operators to choose between constant
strings, such as condition ? "yes" : "no", as well as value == 1 ? "" :
"s":

$ git grep '? "yes" : "no"' | wc -l
258
$ git grep '? "on" : "off"' | wc -l
204
$ git grep '? "enabled" : "disabled"' | wc -l
196
$ git grep '? "" : "s"' | wc -l
25

Additionally, there are some occurences of the same in reverse order,
split to multiple lines, or otherwise not caught by the simple grep.

Add helpers to return the constant strings. Remove existing equivalent
and conflicting functions in i915, cxgb4, and USB core. Further
conversion can be done incrementally.

The main goal here is to abstract recurring patterns, and slightly clean
up the code base by not open coding the ternary operators.

Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: Vishal Kulkarni <vishal@chelsio.com>
Cc: netdev@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

v2: add string-choice.[ch] to not clutter kernel.h and to actually save
space on string constants.

v3: back to static inlines based on Rasmus' feedback

v4: Massaged commit message about space savings to make it less fluffy
based on Rasmus' feedback.

Example of further cleanup possibilities are at [1], to be done
incrementally afterwards.

[1] http://lore.kernel.org/r/20190903133731.2094-2-jani.nikula@intel.com
---
 drivers/gpu/drm/i915/i915_utils.h             | 16 +---------
 .../ethernet/chelsio/cxgb4/cxgb4_debugfs.c    | 12 +------
 drivers/usb/core/config.c                     |  6 +---
 drivers/usb/core/generic.c                    |  6 +---
 include/linux/string-choice.h                 | 31 +++++++++++++++++++
 5 files changed, 35 insertions(+), 36 deletions(-)
 create mode 100644 include/linux/string-choice.h

diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index 562f756da421..794f02a90efe 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -28,6 +28,7 @@
 #include <linux/list.h>
 #include <linux/overflow.h>
 #include <linux/sched.h>
+#include <linux/string-choice.h>
 #include <linux/types.h>
 #include <linux/workqueue.h>
 
@@ -395,21 +396,6 @@ wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms)
 #define MBps(x) KBps(1000 * (x))
 #define GBps(x) ((u64)1000 * MBps((x)))
 
-static inline const char *yesno(bool v)
-{
-	return v ? "yes" : "no";
-}
-
-static inline const char *onoff(bool v)
-{
-	return v ? "on" : "off";
-}
-
-static inline const char *enableddisabled(bool v)
-{
-	return v ? "enabled" : "disabled";
-}
-
 static inline void add_taint_for_CI(unsigned int taint)
 {
 	/*
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
index ae6a47dd7dc9..d9123dae1d00 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
@@ -35,6 +35,7 @@
 #include <linux/seq_file.h>
 #include <linux/debugfs.h>
 #include <linux/string_helpers.h>
+#include <linux/string-choice.h>
 #include <linux/sort.h>
 #include <linux/ctype.h>
 
@@ -2023,17 +2024,6 @@ static const struct file_operations rss_debugfs_fops = {
 /* RSS Configuration.
  */
 
-/* Small utility function to return the strings "yes" or "no" if the supplied
- * argument is non-zero.
- */
-static const char *yesno(int x)
-{
-	static const char *yes = "yes";
-	static const char *no = "no";
-
-	return x ? yes : no;
-}
-
 static int rss_config_show(struct seq_file *seq, void *v)
 {
 	struct adapter *adapter = seq->private;
diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index 151a74a54386..52cee9067eb4 100644
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -10,6 +10,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/device.h>
+#include <linux/string-choice.h>
 #include <asm/byteorder.h>
 #include "usb.h"
 
@@ -19,11 +20,6 @@
 #define USB_MAXCONFIG			8	/* Arbitrary limit */
 
 
-static inline const char *plural(int n)
-{
-	return (n == 1 ? "" : "s");
-}
-
 static int find_next_descriptor(unsigned char *buffer, int size,
     int dt1, int dt2, int *num_skipped)
 {
diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c
index 38f8b3e31762..a784a09794d6 100644
--- a/drivers/usb/core/generic.c
+++ b/drivers/usb/core/generic.c
@@ -21,14 +21,10 @@
 
 #include <linux/usb.h>
 #include <linux/usb/hcd.h>
+#include <linux/string-choice.h>
 #include <uapi/linux/usb/audio.h>
 #include "usb.h"
 
-static inline const char *plural(int n)
-{
-	return (n == 1 ? "" : "s");
-}
-
 static int is_rndis(struct usb_interface_descriptor *desc)
 {
 	return desc->bInterfaceClass == USB_CLASS_COMM
diff --git a/include/linux/string-choice.h b/include/linux/string-choice.h
new file mode 100644
index 000000000000..320b598bd8f0
--- /dev/null
+++ b/include/linux/string-choice.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef __STRING_CHOICE_H__
+#define __STRING_CHOICE_H__
+
+#include <linux/types.h>
+
+static inline const char *yesno(bool v)
+{
+	return v ? "yes" : "no";
+}
+
+static inline const char *onoff(bool v)
+{
+	return v ? "on" : "off";
+}
+
+static inline const char *enableddisabled(bool v)
+{
+	return v ? "enabled" : "disabled";
+}
+
+static inline const char *plural(long v)
+{
+	return v == 1 ? "" : "s";
+}
+
+#endif /* __STRING_CHOICE_H__ */
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

             reply	other threads:[~2019-10-23 13:13 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-23 13:13 Jani Nikula [this message]
2019-10-23 13:13 ` [Intel-gfx] [PATCH v4] string-choice: add yesno(), onoff(), enableddisabled(), plural() helpers Jani Nikula
2019-10-23 13:21 ` Rasmus Villemoes
2019-10-23 13:21   ` [Intel-gfx] " Rasmus Villemoes
2019-10-23 13:26   ` Jani Nikula
2019-10-23 13:26     ` [Intel-gfx] " Jani Nikula
2019-10-23 13:26     ` Jani Nikula
2019-10-23 22:56 ` Andrew Morton
2019-10-23 22:56   ` [Intel-gfx] " Andrew Morton
2019-10-23 23:46   ` Joe Perches
2019-10-23 23:46     ` [Intel-gfx] " Joe Perches
2019-10-24  7:32   ` Jani Nikula
2019-10-24  7:32     ` [Intel-gfx] " Jani Nikula
2019-10-24  7:32     ` Jani Nikula
2019-10-24  7:40   ` Rasmus Villemoes
2019-10-24  7:40     ` [Intel-gfx] " Rasmus Villemoes
2019-10-24  7:57     ` Rasmus Villemoes
2019-10-24  7:57       ` [Intel-gfx] " Rasmus Villemoes
2019-10-24  7:57       ` Rasmus Villemoes
2019-10-24  2:21 ` ✗ Fi.CI.BUILD: failure for string-choice: add yesno(), onoff(), enableddisabled(), plural() helpers (rev2) Patchwork
2019-10-24  2:21   ` [Intel-gfx] " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191023131308.9420-1-jani.nikula@intel.com \
    --to=jani.nikula@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=julia.lawall@lip6.fr \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=netdev@vger.kernel.org \
    --cc=rodrigo.vivi@intel.com \
    --cc=vishal@chelsio.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.