[PATCH v2 0/7] lib: string: add functions to case-convert strings

[PATCH v2 0/7] lib: string: add functions to case-convert strings

[Markus Mayer]

This series introduces a family of generic string case conversion
functions. This kind of functionality is needed in several places in
the kernel. Right now, everybody seems to be implementing their own
copy of this functionality.

Based on the discussion of the previous version of this series[1] and
the use cases found in the kernel, it does look like having several
flavours of case conversion functions is beneficial. The use cases fall
into three categories:
    - copying a string and converting the case while specifying a
      maximum length to mimic strncpy()
    - copying a string and converting the case without specifying a
      length to mimic strcpy()
    - converting the case of a string in-place (i.e. modifying the
      string that was passed in)

Consequently, I am proposing these new functions:
    char *strncpytoupper(char *dst, const char *src, size_t len);
    char *strncpytolower(char *dst, const char *src, size_t len);
    char *strcpytoupper(char *dst, const char *src);
    char *strcpytolower(char *dst, const char *src);
    char *strtoupper(char *s);
    char *strtolower(char *s);
They all return a pointer to the terminating '\0' in the destination
string (for strtoupper() and strtolower() that is "s").

Several drivers are being modified to make use of the functions above.
Another driver that also makes use of this functionality will be
submitted upstream shortly, which prompted this whole exercise.

The changes made here have been compile-tested, but not tried out, due
to lack of required hardware.

Changes since v1:
  - expanded strtolower() into a family of functions that cover use
    cases when a length argument is or isn't required and that support
    copying the string into a new buffer or changing it in-place 
  - changed the function semantics to return a pointer to the
    terminating '\0' character of the modified string
  - added strtoupper() functionality mirroring the above
  - dropped the ACPICA patch, since that code is OS independent and
    can't rely on a Linux library function (see [2])
  - Added two new patches replacing strtoupper() implementations

[1] https://lkml.org/lkml/2016/6/30/727
[2] https://lkml.org/lkml/2016/7/1/9

Markus Mayer (7):
  lib: string: add functions str[n]cpytolower()/str[n]cpytoupper()
  drm/nouveau/core: make use of new strncpytolower() function
  ACPI / device_sysfs: make use of new strtolower() function
  staging: speakup: replace spk_strlwr() with strncpytolower()
  iscsi-target: replace iscsi_initiatorname_tolower() with
    strcpytolower()
  drm/nouveau/fifo/gk104: make use of new strcpytoupper() function
  power_supply: make use of new strcpytoupper() function

 drivers/acpi/device_sysfs.c                      |  4 +-
 drivers/gpu/drm/nouveau/nvkm/core/firmware.c     |  9 +----
 drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c |  5 +--
 drivers/power/power_supply_sysfs.c               | 13 +++----
 drivers/staging/speakup/kobjects.c               |  3 +-
 drivers/staging/speakup/main.c                   |  3 +-
 drivers/staging/speakup/speakup.h                |  1 -
 drivers/staging/speakup/varhandlers.c            | 12 ------
 drivers/target/iscsi/iscsi_target_nego.c         | 17 +--------
 include/linux/string.h                           | 48 ++++++++++++++++++++++++
 lib/string.c                                     | 40 ++++++++++++++++++++
 11 files changed, 100 insertions(+), 55 deletions(-)

-- 
2.7.4

[PATCH v2 1/7] lib: string: add functions to case-convert strings

[Markus Mayer]

Add a collection of generic functions to convert strings to lowercase
or uppercase.

Changing the case of a string (with or without copying it first) seems
to be a recurring requirement in the kernel that is currently being
solved by several duplicated implementations doing the same thing. This
change aims at reducing this code duplication.

The new functions are
    char *strncpytoupper(char *dst, const char *src, size_t len);
    char *strncpytolower(char *dst, const char *src, size_t len);
    char *strcpytoupper(char *dst, const char *src);
    char *strcpytolower(char *dst, const char *src);
    char *strtoupper(char *s);
    char *strtolower(char *s);

The "str[n]cpyto*" versions of the function take a destination string
and a source string as arguments. The "strncpyto*" versions
additionally take a length argument like strncpy() itself. Lastly, the
strto* functions take a single string argument and modify the passed-in
string.

All functions return a pointer to the terminating '\0' character in the
modified string ("dst" or "s", respectively).

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 include/linux/string.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 lib/string.c           | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index 26b6f6a..c58d510 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -116,6 +116,8 @@ extern void * memchr(const void *,int,__kernel_size_t);
 #endif
 void *memchr_inv(const void *s, int c, size_t n);
 char *strreplace(char *s, char old, char new);
+char *strncpytoupper(char *dst, const char *src, size_t len);
+char *strncpytolower(char *dst, const char *src, size_t len);
 
 extern void kfree_const(const void *x);
 
@@ -169,4 +171,50 @@ static inline const char *kbasename(const char *path)
 	return tail ? tail + 1 : path;
 }
 
+/**
+ * strcpytoupper - Copy string and convert to uppercase.
+ * @dst: The buffer to store the result.
+ * @src: The string to convert to uppercase.
+ *
+ * Returns pointer to terminating '\0' in @dst.
+ */
+static inline char *strcpytoupper(char *dst, const char *src)
+{
+	return strncpytoupper(dst, src, 0);
+}
+
+/**
+ * strcpytolower - Copy string and convert to lowercase.
+ * @dst: The buffer to store the result.
+ * @src: The string to convert to lowercase.
+ *
+ * Returns pointer to terminating '\0' in @dst.
+ */
+static inline char *strcpytolower(char *dst, const char *src)
+{
+	return strncpytolower(dst, src, 0);
+}
+
+/**
+ * strtoupper - Convert string to uppercase.
+ * @s: The string to operate on.
+ *
+ * Returns pointer to terminating '\0' in @s.
+ */
+static inline char *strtoupper(char *s)
+{
+	return strncpytoupper(s, s, 0);
+}
+
+/**
+ * strtolower - Convert string to lowercase.
+ * @s: The string to operate on.
+ *
+ * Returns pointer to terminating '\0' in @s.
+ */
+static inline char *strtolower(char *s)
+{
+	return strncpytolower(s, s, 0);
+}
+
 #endif /* _LINUX_STRING_H_ */
diff --git a/lib/string.c b/lib/string.c
index ed83562..900f357 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -952,3 +952,45 @@ char *strreplace(char *s, char old, char new)
 	return s;
 }
 EXPORT_SYMBOL(strreplace);
+
+/**
+ * strncpytoupper - Copy a length-limited string and convert to uppercase.
+ * @dst: The buffer to store the result.
+ * @src: The string to convert to uppercase.
+ * @len: Maximum string length. May be 0 to set no limit.
+ *
+ * Returns pointer to terminating '\0' in @dst.
+ */
+char *strncpytoupper(char *dst, const char *src, size_t len)
+{
+	size_t i;
+
+	for (i = 0; src[i] != '\0' && (i < len || !len); i++)
+		dst[i] = toupper(src[i]);
+	if (i < len || !len)
+		dst[i] = '\0';
+
+	return dst + i;
+}
+EXPORT_SYMBOL(strncpytoupper);
+
+/**
+ * strncpytolower - Copy a length-limited string and convert to lowercase.
+ * @dst: The buffer to store the result.
+ * @src: The string to convert to lowercase.
+ * @len: Maximum string length. May be 0 to set no limit.
+ *
+ * Returns pointer to terminating '\0' in @dst.
+ */
+char *strncpytolower(char *dst, const char *src, size_t len)
+{
+	size_t i;
+
+	for (i = 0; src[i] != '\0' && (i < len || !len); i++)
+		dst[i] = tolower(src[i]);
+	if (i < len || !len)
+		dst[i] = '\0';
+
+	return dst + i;
+}
+EXPORT_SYMBOL(strncpytolower);
-- 
2.7.4

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH v2 1/7] lib: string: add functions to case-convert strings

[Markus Mayer]

Add a collection of generic functions to convert strings to lowercase
or uppercase.

Changing the case of a string (with or without copying it first) seems
to be a recurring requirement in the kernel that is currently being
solved by several duplicated implementations doing the same thing. This
change aims at reducing this code duplication.

The new functions are
    char *strncpytoupper(char *dst, const char *src, size_t len);
    char *strncpytolower(char *dst, const char *src, size_t len);
    char *strcpytoupper(char *dst, const char *src);
    char *strcpytolower(char *dst, const char *src);
    char *strtoupper(char *s);
    char *strtolower(char *s);

The "str[n]cpyto*" versions of the function take a destination string
and a source string as arguments. The "strncpyto*" versions
additionally take a length argument like strncpy() itself. Lastly, the
strto* functions take a single string argument and modify the passed-in
string.

All functions return a pointer to the terminating '\0' character in the
modified string ("dst" or "s", respectively).

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 include/linux/string.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 lib/string.c           | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index 26b6f6a..c58d510 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -116,6 +116,8 @@ extern void * memchr(const void *,int,__kernel_size_t);
 #endif
 void *memchr_inv(const void *s, int c, size_t n);
 char *strreplace(char *s, char old, char new);
+char *strncpytoupper(char *dst, const char *src, size_t len);
+char *strncpytolower(char *dst, const char *src, size_t len);
 
 extern void kfree_const(const void *x);
 
@@ -169,4 +171,50 @@ static inline const char *kbasename(const char *path)
 	return tail ? tail + 1 : path;
 }
 
+/**
+ * strcpytoupper - Copy string and convert to uppercase.
+ * @dst: The buffer to store the result.
+ * @src: The string to convert to uppercase.
+ *
+ * Returns pointer to terminating '\0' in @dst.
+ */
+static inline char *strcpytoupper(char *dst, const char *src)
+{
+	return strncpytoupper(dst, src, 0);
+}
+
+/**
+ * strcpytolower - Copy string and convert to lowercase.
+ * @dst: The buffer to store the result.
+ * @src: The string to convert to lowercase.
+ *
+ * Returns pointer to terminating '\0' in @dst.
+ */
+static inline char *strcpytolower(char *dst, const char *src)
+{
+	return strncpytolower(dst, src, 0);
+}
+
+/**
+ * strtoupper - Convert string to uppercase.
+ * @s: The string to operate on.
+ *
+ * Returns pointer to terminating '\0' in @s.
+ */
+static inline char *strtoupper(char *s)
+{
+	return strncpytoupper(s, s, 0);
+}
+
+/**
+ * strtolower - Convert string to lowercase.
+ * @s: The string to operate on.
+ *
+ * Returns pointer to terminating '\0' in @s.
+ */
+static inline char *strtolower(char *s)
+{
+	return strncpytolower(s, s, 0);
+}
+
 #endif /* _LINUX_STRING_H_ */
diff --git a/lib/string.c b/lib/string.c
index ed83562..900f357 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -952,3 +952,45 @@ char *strreplace(char *s, char old, char new)
 	return s;
 }
 EXPORT_SYMBOL(strreplace);
+
+/**
+ * strncpytoupper - Copy a length-limited string and convert to uppercase.
+ * @dst: The buffer to store the result.
+ * @src: The string to convert to uppercase.
+ * @len: Maximum string length. May be 0 to set no limit.
+ *
+ * Returns pointer to terminating '\0' in @dst.
+ */
+char *strncpytoupper(char *dst, const char *src, size_t len)
+{
+	size_t i;
+
+	for (i = 0; src[i] != '\0' && (i < len || !len); i++)
+		dst[i] = toupper(src[i]);
+	if (i < len || !len)
+		dst[i] = '\0';
+
+	return dst + i;
+}
+EXPORT_SYMBOL(strncpytoupper);
+
+/**
+ * strncpytolower - Copy a length-limited string and convert to lowercase.
+ * @dst: The buffer to store the result.
+ * @src: The string to convert to lowercase.
+ * @len: Maximum string length. May be 0 to set no limit.
+ *
+ * Returns pointer to terminating '\0' in @dst.
+ */
+char *strncpytolower(char *dst, const char *src, size_t len)
+{
+	size_t i;
+
+	for (i = 0; src[i] != '\0' && (i < len || !len); i++)
+		dst[i] = tolower(src[i]);
+	if (i < len || !len)
+		dst[i] = '\0';
+
+	return dst + i;
+}
+EXPORT_SYMBOL(strncpytolower);
-- 
2.7.4

[PATCH v2 2/7] drm/nouveau/core: make use of new strncpytolower() function

[Markus Mayer]

Call strncpytolower() rather than copying the string explicitly and
then walking it to convert it to lowercase.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 drivers/gpu/drm/nouveau/nvkm/core/firmware.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
index 34ecd4a..6f3314f 100644
--- a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
+++ b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
@@ -36,16 +36,10 @@ nvkm_firmware_get(struct nvkm_device *device, const char *fwname,
 {
 	char f[64];
 	char cname[16];
-	int i;
 
 	/* Convert device name to lowercase */
-	strncpy(cname, device->chip->name, sizeof(cname));
+	strncpytolower(cname, device->chip->name, sizeof(cname));
 	cname[sizeof(cname) - 1] = '\0';
-	i = strlen(cname);
-	while (i) {
-		--i;
-		cname[i] = tolower(cname[i]);
-	}
 
 	snprintf(f, sizeof(f), "nvidia/%s/%s.bin", cname, fwname);
 	return request_firmware(fw, f, device->dev);
-- 
2.7.4

[PATCH v2 2/7] drm/nouveau/core: make use of new strncpytolower() function

[Markus Mayer]

Call strncpytolower() rather than copying the string explicitly and
then walking it to convert it to lowercase.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 drivers/gpu/drm/nouveau/nvkm/core/firmware.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
index 34ecd4a..6f3314f 100644
--- a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
+++ b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
@@ -36,16 +36,10 @@ nvkm_firmware_get(struct nvkm_device *device, const char *fwname,
 {
 	char f[64];
 	char cname[16];
-	int i;
 
 	/* Convert device name to lowercase */
-	strncpy(cname, device->chip->name, sizeof(cname));
+	strncpytolower(cname, device->chip->name, sizeof(cname));
 	cname[sizeof(cname) - 1] = '\0';
-	i = strlen(cname);
-	while (i) {
-		--i;
-		cname[i] = tolower(cname[i]);
-	}
 
 	snprintf(f, sizeof(f), "nvidia/%s/%s.bin", cname, fwname);
 	return request_firmware(fw, f, device->dev);
-- 
2.7.4

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH v2 3/7] ACPI / device_sysfs: make use of new strtolower() function

[Markus Mayer]

Call strtolower() rather than walking the string explicitly to convert
it to lowercase.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---

Rafael, I left off your ACK, since the implementation of my function changed
somewhat (no check for NULL, return value is different). Please let me know
if you are still okay with this.

 drivers/acpi/device_sysfs.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
index 7b2c48f..1db38c7 100644
--- a/drivers/acpi/device_sysfs.c
+++ b/drivers/acpi/device_sysfs.c
@@ -200,12 +200,10 @@ static int create_of_modalias(struct acpi_device *acpi_dev, char *modalias,
 	const union acpi_object *of_compatible, *obj;
 	int len, count;
 	int i, nval;
-	char *c;
 
 	acpi_get_name(acpi_dev->handle, ACPI_SINGLE_NAME, &buf);
 	/* DT strings are all in lower case */
-	for (c = buf.pointer; *c != '\0'; c++)
-		*c = tolower(*c);
+	strtolower(buf.pointer);
 
 	len = snprintf(modalias, size, "of:N%sT", (char *)buf.pointer);
 	ACPI_FREE(buf.pointer);
-- 
2.7.4

[PATCH v2 4/7] staging: speakup: replace spk_strlwr() with strncpytolower()

[Markus Mayer]

After introducing generic strntolower() and strtolower(), spk_strlwr()
is no longer needed.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---

Samuel, I left off your ACK, since the implementation of my function changed
somewhat (no check for NULL, return value is different). Please let me know
if you are still okay with this.

 drivers/staging/speakup/kobjects.c    |  3 +--
 drivers/staging/speakup/main.c        |  3 ++-
 drivers/staging/speakup/speakup.h     |  1 -
 drivers/staging/speakup/varhandlers.c | 12 ------------
 4 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/speakup/kobjects.c b/drivers/staging/speakup/kobjects.c
index 528cbdc..3007325 100644
--- a/drivers/staging/speakup/kobjects.c
+++ b/drivers/staging/speakup/kobjects.c
@@ -387,11 +387,10 @@ static ssize_t synth_store(struct kobject *kobj, struct kobj_attribute *attr,
 	len = strlen(buf);
 	if (len < 2 || len > 9)
 		return -EINVAL;
-	strncpy(new_synth_name, buf, len);
+	strncpytolower(new_synth_name, buf, len);
 	if (new_synth_name[len - 1] == '\n')
 		len--;
 	new_synth_name[len] = '\0';
-	spk_strlwr(new_synth_name);
 	if ((synth != NULL) && (!strcmp(new_synth_name, synth->name))) {
 		pr_warn("%s already in use\n", new_synth_name);
 	} else if (synth_init(new_synth_name) != 0) {
diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index 97ca4ec..970f568 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -2314,7 +2314,8 @@ static int __init speakup_init(void)
 	spk_initialize_msgs();	/* Initialize arrays for i18n. */
 	spk_reset_default_chars();
 	spk_reset_default_chartab();
-	spk_strlwr(synth_name);
+	if (synth_name)
+		strtolower(synth_name);
 	spk_vars[0].u.n.high = vc->vc_cols;
 	for (var = spk_vars; var->var_id != MAXVARS; var++)
 		speakup_register_var(var);
diff --git a/drivers/staging/speakup/speakup.h b/drivers/staging/speakup/speakup.h
index df74c91..4725785 100644
--- a/drivers/staging/speakup/speakup.h
+++ b/drivers/staging/speakup/speakup.h
@@ -50,7 +50,6 @@ void synth_insert_next_index(int sent_num);
 void spk_reset_index_count(int sc);
 void spk_get_index_count(int *linecount, int *sentcount);
 int spk_set_key_info(const u_char *key_info, u_char *k_buffer);
-char *spk_strlwr(char *s);
 char *spk_s2uchar(char *start, char *dest);
 int speakup_kobj_init(void);
 void speakup_kobj_exit(void);
diff --git a/drivers/staging/speakup/varhandlers.c b/drivers/staging/speakup/varhandlers.c
index e1393d2..a1af222 100644
--- a/drivers/staging/speakup/varhandlers.c
+++ b/drivers/staging/speakup/varhandlers.c
@@ -309,18 +309,6 @@ int spk_set_mask_bits(const char *input, const int which, const int how)
 	return 0;
 }
 
-char *spk_strlwr(char *s)
-{
-	char *p;
-
-	if (!s)
-		return NULL;
-
-	for (p = s; *p; p++)
-		*p = tolower(*p);
-	return s;
-}
-
 char *spk_s2uchar(char *start, char *dest)
 {
 	int val;
-- 
2.7.4

[PATCH v2 5/7] iscsi-target: replace iscsi_initiatorname_tolower() with strtolower()

[Markus Mayer]

After introducing generic strtolower(), iscsi_initiatorname_tolower() is
no longer needed.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 drivers/target/iscsi/iscsi_target_nego.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_nego.c b/drivers/target/iscsi/iscsi_target_nego.c
index 89d34bd..fa20638 100644
--- a/drivers/target/iscsi/iscsi_target_nego.c
+++ b/drivers/target/iscsi/iscsi_target_nego.c
@@ -987,21 +987,6 @@ static int iscsi_target_do_login(struct iscsi_conn *conn, struct iscsi_login *lo
 	return 0;
 }
 
-static void iscsi_initiatorname_tolower(
-	char *param_buf)
-{
-	char *c;
-	u32 iqn_size = strlen(param_buf), i;
-
-	for (i = 0; i < iqn_size; i++) {
-		c = &param_buf[i];
-		if (!isupper(*c))
-			continue;
-
-		*c = tolower(*c);
-	}
-}
-
 /*
  * Processes the first Login Request..
  */
@@ -1075,7 +1060,7 @@ int iscsi_target_locate_portal(
 	 * RFC-3720 3.2.6.1. section c) that says that iSCSI IQNs
 	 * are NOT case sensitive.
 	 */
-	iscsi_initiatorname_tolower(i_buf);
+	strtolower(i_buf);
 
 	if (!s_buf) {
 		if (!login->leading_connection)
-- 
2.7.4

[PATCH v2 6/7] drm/nouveau/fifo/gk104: make use of new strcpytoupper() function

[Markus Mayer]

Call strcpytoupper() rather than copying the string explicitly and then
walking it to convert it to uppercase.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c
index 743f3a1..8d01032 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c
@@ -332,10 +332,7 @@ gk104_fifo_intr_fault(struct gk104_fifo *fifo, int unit)
 		enum nvkm_devidx engidx = nvkm_top_fault(device->top, unit);
 		if (engidx < NVKM_SUBDEV_NR) {
 			const char *src = nvkm_subdev_name[engidx];
-			char *dst = en;
-			do {
-				*dst++ = toupper(*src++);
-			} while(*src);
+			strcpytoupper(en, src);
 			engine = nvkm_device_engine(device, engidx);
 		}
 	} else {
-- 
2.7.4

[PATCH v2 6/7] drm/nouveau/fifo/gk104: make use of new strcpytoupper() function

[Markus Mayer]

Call strcpytoupper() rather than copying the string explicitly and then
walking it to convert it to uppercase.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c
index 743f3a1..8d01032 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c
@@ -332,10 +332,7 @@ gk104_fifo_intr_fault(struct gk104_fifo *fifo, int unit)
 		enum nvkm_devidx engidx = nvkm_top_fault(device->top, unit);
 		if (engidx < NVKM_SUBDEV_NR) {
 			const char *src = nvkm_subdev_name[engidx];
-			char *dst = en;
-			do {
-				*dst++ = toupper(*src++);
-			} while(*src);
+			strcpytoupper(en, src);
 			engine = nvkm_device_engine(device, engidx);
 		}
 	} else {
-- 
2.7.4

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH v2 7/7] power_supply: make use of new strcpytoupper() function

[Markus Mayer]

Call strcpytoupper() rather than walking the string explicitly to
convert it to uppercase.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 drivers/power/power_supply_sysfs.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/power/power_supply_sysfs.c b/drivers/power/power_supply_sysfs.c
index 80fed98..20fdcc5 100644
--- a/drivers/power/power_supply_sysfs.c
+++ b/drivers/power/power_supply_sysfs.c
@@ -256,19 +256,16 @@ void power_supply_init_attrs(struct device_type *dev_type)
 
 static char *kstruprdup(const char *str, gfp_t gfp)
 {
-	char *ret, *ustr;
+	char *ustr;
 
-	ustr = ret = kmalloc(strlen(str) + 1, gfp);
+	ustr = kmalloc(strlen(str) + 1, gfp);
 
-	if (!ret)
+	if (!ustr)
 		return NULL;
 
-	while (*str)
-		*ustr++ = toupper(*str++);
+	strcpytoupper(ustr, str);
 
-	*ustr = 0;
-
-	return ret;
+	return ustr;
 }
 
 int power_supply_uevent(struct device *dev, struct kobj_uevent_env *env)
-- 
2.7.4

Re: [PATCH v2 0/7] lib: string: add functions to case-convert strings

[Joe Perches]

On Tue, 2016-07-05 at 13:47 -0700, Markus Mayer wrote:
> This series introduces a family of generic string case conversion
> functions. This kind of functionality is needed in several places in
> the kernel. Right now, everybody seems to be implementing their own
> copy of this functionality.
> 
> Based on the discussion of the previous version of this series[1] and
> the use cases found in the kernel, it does look like having several
> flavours of case conversion functions is beneficial. The use cases fall
> into three categories:
>     - copying a string and converting the case while specifying a
>       maximum length to mimic strncpy()
>     - copying a string and converting the case without specifying a
>       length to mimic strcpy()
>     - converting the case of a string in-place (i.e. modifying the
>       string that was passed in)
> 
> Consequently, I am proposing these new functions:
>     char *strncpytoupper(char *dst, const char *src, size_t len);
>     char *strncpytolower(char *dst, const char *src, size_t len);
>     char *strcpytoupper(char *dst, const char *src);
>     char *strcpytolower(char *dst, const char *src);
>     char *strtoupper(char *s);
>     char *strtolower(char *s);

I think there isn't much value in anything other
than strto<upper|lower>.

Using str[n]cpy followed by strto<upper|lower> is
pretty obvious and rarely used anyway.

Re: [PATCH v2 0/7] lib: string: add functions to case-convert strings

[Markus Mayer]

On 5 July 2016 at 15:14, Joe Perches <joe@perches.com> wrote:
> On Tue, 2016-07-05 at 13:47 -0700, Markus Mayer wrote:
>> This series introduces a family of generic string case conversion
>> functions. This kind of functionality is needed in several places in
>> the kernel. Right now, everybody seems to be implementing their own
>> copy of this functionality.
>>
>> Based on the discussion of the previous version of this series[1] and
>> the use cases found in the kernel, it does look like having several
>> flavours of case conversion functions is beneficial. The use cases fall
>> into three categories:
>>     - copying a string and converting the case while specifying a
>>       maximum length to mimic strncpy()
>>     - copying a string and converting the case without specifying a
>>       length to mimic strcpy()
>>     - converting the case of a string in-place (i.e. modifying the
>>       string that was passed in)
>>
>> Consequently, I am proposing these new functions:
>>     char *strncpytoupper(char *dst, const char *src, size_t len);
>>     char *strncpytolower(char *dst, const char *src, size_t len);
>>     char *strcpytoupper(char *dst, const char *src);
>>     char *strcpytolower(char *dst, const char *src);
>>     char *strtoupper(char *s);
>>     char *strtolower(char *s);
>
> I think there isn't much value in anything other
> than strto<upper|lower>.
>
> Using str[n]cpy followed by strto<upper|lower> is
> pretty obvious and rarely used anyway.

First time around, folks were proposing the "copy" variants when I
submitted just strtolower() by itself[1]. They just asked for source
and destination parameters to strtolower(), but looking at the use
cases that wouldn't have worked so well. Hence it evolved into these 6
functions.

Here's a breakdown of how the functions are being used (patches 2-7),
see also [2]:

Patch 2: strncpytolower()
Patch 3: strtolower()
Patch 4: strncpytolower() and strtolower()
Patch 5: strtolower()
Patch 6: strcpytoupper()
Patch 7: strcpytoupper()

So it does look like the copy + change case variant is more frequently
used than just strto<upper|lower>.

Regards,
-Markus

[1] https://lkml.org/lkml/2016/7/1/652
[2] https://lkml.org/lkml/2016/7/5/542
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH v2 0/7] lib: string: add functions to case-convert strings

[Markus Mayer]

On 5 July 2016 at 15:14, Joe Perches <joe@perches.com> wrote:
> On Tue, 2016-07-05 at 13:47 -0700, Markus Mayer wrote:
>> This series introduces a family of generic string case conversion
>> functions. This kind of functionality is needed in several places in
>> the kernel. Right now, everybody seems to be implementing their own
>> copy of this functionality.
>>
>> Based on the discussion of the previous version of this series[1] and
>> the use cases found in the kernel, it does look like having several
>> flavours of case conversion functions is beneficial. The use cases fall
>> into three categories:
>>     - copying a string and converting the case while specifying a
>>       maximum length to mimic strncpy()
>>     - copying a string and converting the case without specifying a
>>       length to mimic strcpy()
>>     - converting the case of a string in-place (i.e. modifying the
>>       string that was passed in)
>>
>> Consequently, I am proposing these new functions:
>>     char *strncpytoupper(char *dst, const char *src, size_t len);
>>     char *strncpytolower(char *dst, const char *src, size_t len);
>>     char *strcpytoupper(char *dst, const char *src);
>>     char *strcpytolower(char *dst, const char *src);
>>     char *strtoupper(char *s);
>>     char *strtolower(char *s);
>
> I think there isn't much value in anything other
> than strto<upper|lower>.
>
> Using str[n]cpy followed by strto<upper|lower> is
> pretty obvious and rarely used anyway.

First time around, folks were proposing the "copy" variants when I
submitted just strtolower() by itself[1]. They just asked for source
and destination parameters to strtolower(), but looking at the use
cases that wouldn't have worked so well. Hence it evolved into these 6
functions.

Here's a breakdown of how the functions are being used (patches 2-7),
see also [2]:

Patch 2: strncpytolower()
Patch 3: strtolower()
Patch 4: strncpytolower() and strtolower()
Patch 5: strtolower()
Patch 6: strcpytoupper()
Patch 7: strcpytoupper()

So it does look like the copy + change case variant is more frequently
used than just strto<upper|lower>.

Regards,
-Markus

[1] https://lkml.org/lkml/2016/7/1/652
[2] https://lkml.org/lkml/2016/7/5/542

Re: [PATCH v2 0/7] lib: string: add functions to case-convert strings

[Joe Perches]

On Tue, 2016-07-05 at 15:36 -0700, Markus Mayer wrote:
> On 5 July 2016 at 15:14, Joe Perches <joe@perches.com> wrote:
> > On Tue, 2016-07-05 at 13:47 -0700, Markus Mayer wrote:
> > > This series introduces a family of generic string case conversion
> > > functions. This kind of functionality is needed in several places in
> > > the kernel. Right now, everybody seems to be implementing their own
> > > copy of this functionality.
> > > 
> > > Based on the discussion of the previous version of this series[1] and
> > > the use cases found in the kernel, it does look like having several
> > > flavours of case conversion functions is beneficial. The use cases fall
> > > into three categories:
> > >     - copying a string and converting the case while specifying a
> > >       maximum length to mimic strncpy()
> > >     - copying a string and converting the case without specifying a
> > >       length to mimic strcpy()
> > >     - converting the case of a string in-place (i.e. modifying the
> > >       string that was passed in)
> > > 
> > > Consequently, I am proposing these new functions:
> > >     char *strncpytoupper(char *dst, const char *src, size_t len);
> > >     char *strncpytolower(char *dst, const char *src, size_t len);
> > >     char *strcpytoupper(char *dst, const char *src);
> > >     char *strcpytolower(char *dst, const char *src);
> > >     char *strtoupper(char *s);
> > >     char *strtolower(char *s);
> > I think there isn't much value in anything other
> > than strto.
> > 
> > Using str[n]cpy followed by strto is
> > pretty obvious and rarely used anyway.
> First time around, folks were proposing the "copy" variants when I
> submitted just strtolower() by itself[1]. They just asked for source
> and destination parameters to strtolower(), but looking at the use
> cases that wouldn't have worked so well. Hence it evolved into these 6
> functions.
> 
> Here's a breakdown of how the functions are being used (patches 2-7),
> see also [2]:
> 
> Patch 2: strncpytolower()
> Patch 3: strtolower()
> Patch 4: strncpytolower() and strtolower()
> Patch 5: strtolower()
> Patch 6: strcpytoupper()
> Patch 7: strcpytoupper()
> 
> So it does look like the copy + change case variant is more frequently
> used than just strto.

Are these functions useful?  <shrug> Not to me, not so much.

None of the functions would have the strcpy performance of
the arch / asm
versions of strcpy and the savings in overall
code isn't significant (or
measured?).

Of course none of the uses are runtime performance important.

This patch also adds always compiled functions that aren't used
in many .configs.

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/7] lib: string: add functions to case-convert strings

[Joe Perches]

On Tue, 2016-07-05 at 15:36 -0700, Markus Mayer wrote:
> On 5 July 2016 at 15:14, Joe Perches <joe@perches.com> wrote:
> > On Tue, 2016-07-05 at 13:47 -0700, Markus Mayer wrote:
> > > This series introduces a family of generic string case conversion
> > > functions. This kind of functionality is needed in several places in
> > > the kernel. Right now, everybody seems to be implementing their own
> > > copy of this functionality.
> > > 
> > > Based on the discussion of the previous version of this series[1] and
> > > the use cases found in the kernel, it does look like having several
> > > flavours of case conversion functions is beneficial. The use cases fall
> > > into three categories:
> > >     - copying a string and converting the case while specifying a
> > >       maximum length to mimic strncpy()
> > >     - copying a string and converting the case without specifying a
> > >       length to mimic strcpy()
> > >     - converting the case of a string in-place (i.e. modifying the
> > >       string that was passed in)
> > > 
> > > Consequently, I am proposing these new functions:
> > >     char *strncpytoupper(char *dst, const char *src, size_t len);
> > >     char *strncpytolower(char *dst, const char *src, size_t len);
> > >     char *strcpytoupper(char *dst, const char *src);
> > >     char *strcpytolower(char *dst, const char *src);
> > >     char *strtoupper(char *s);
> > >     char *strtolower(char *s);
> > I think there isn't much value in anything other
> > than strto.
> > 
> > Using str[n]cpy followed by strto is
> > pretty obvious and rarely used anyway.
> First time around, folks were proposing the "copy" variants when I
> submitted just strtolower() by itself[1]. They just asked for source
> and destination parameters to strtolower(), but looking at the use
> cases that wouldn't have worked so well. Hence it evolved into these 6
> functions.
> 
> Here's a breakdown of how the functions are being used (patches 2-7),
> see also [2]:
> 
> Patch 2: strncpytolower()
> Patch 3: strtolower()
> Patch 4: strncpytolower() and strtolower()
> Patch 5: strtolower()
> Patch 6: strcpytoupper()
> Patch 7: strcpytoupper()
> 
> So it does look like the copy + change case variant is more frequently
> used than just strto.

Are these functions useful?  <shrug> Not to me, not so much.

None of the functions would have the strcpy performance of
the arch / asm
versions of strcpy and the savings in overall
code isn't significant (or
measured?).

Of course none of the uses are runtime performance important.

This patch also adds always compiled functions that aren't used
in many .configs.

Re: [PATCH v2 0/7] lib: string: add functions to case-convert strings

[Markus Mayer]

On 5 July 2016 at 15:56, Joe Perches <joe@perches.com> wrote:
> On Tue, 2016-07-05 at 15:36 -0700, Markus Mayer wrote:
>> On 5 July 2016 at 15:14, Joe Perches <joe@perches.com> wrote:
>> > On Tue, 2016-07-05 at 13:47 -0700, Markus Mayer wrote:
>> > > This series introduces a family of generic string case conversion
>> > > functions. This kind of functionality is needed in several places in
>> > > the kernel. Right now, everybody seems to be implementing their own
>> > > copy of this functionality.
>> > >
>> > > Based on the discussion of the previous version of this series[1] and
>> > > the use cases found in the kernel, it does look like having several
>> > > flavours of case conversion functions is beneficial. The use cases fall
>> > > into three categories:
>> > >     - copying a string and converting the case while specifying a
>> > >       maximum length to mimic strncpy()
>> > >     - copying a string and converting the case without specifying a
>> > >       length to mimic strcpy()
>> > >     - converting the case of a string in-place (i.e. modifying the
>> > >       string that was passed in)
>> > >
>> > > Consequently, I am proposing these new functions:
>> > >     char *strncpytoupper(char *dst, const char *src, size_t len);
>> > >     char *strncpytolower(char *dst, const char *src, size_t len);
>> > >     char *strcpytoupper(char *dst, const char *src);
>> > >     char *strcpytolower(char *dst, const char *src);
>> > >     char *strtoupper(char *s);
>> > >     char *strtolower(char *s);
>> > I think there isn't much value in anything other
>> > than strto.
>> >
>> > Using str[n]cpy followed by strto is
>> > pretty obvious and rarely used anyway.
>> First time around, folks were proposing the "copy" variants when I
>> submitted just strtolower() by itself[1]. They just asked for source
>> and destination parameters to strtolower(), but looking at the use
>> cases that wouldn't have worked so well. Hence it evolved into these 6
>> functions.
>>
>> Here's a breakdown of how the functions are being used (patches 2-7),
>> see also [2]:
>>
>> Patch 2: strncpytolower()
>> Patch 3: strtolower()
>> Patch 4: strncpytolower() and strtolower()
>> Patch 5: strtolower()
>> Patch 6: strcpytoupper()
>> Patch 7: strcpytoupper()
>>
>> So it does look like the copy + change case variant is more frequently
>> used than just strto.
>
> Are these functions useful?  <shrug> Not to me, not so much.

The use cases do exist. I'll leave it up to the maintainers to decide
whether duplicate implementations or potentially unused generic
functions are to be preferred.

What I do know is that I have a driver in the wings that also needs a
strolower() implementation. If this ends up not being accepted, I'll
have to add yet another driver-local strtolower() implementation to
the kernel. But if that's the decision then so be it.

> None of the functions would have the strcpy performance of
> the arch / asm
> versions of strcpy and the savings in overall
> code isn't significant (or
> measured?).

100% agreed. These functions won't set any speed records. Keep in mind
also that in 4 out of the 6 cases where I replaced local
implementations of "strcpytoXXX", the code was doing essentially the
same I am proposing here (no assembly code). Only 2 of the 6 called
strncpy() before, benefiting from optimized assembly implementations.
But they still had to walk the string explicitly afterwards to convert
the case, which probably means the overall speed won't change that
much using the functions proposed here.

> Of course none of the uses are runtime performance important.

That is also very true.

> This patch also adds always compiled functions that aren't used
> in many .configs.

It adds 2 functions (strncpytolower() and strncpytoupper()). The other
4 are static inline one-liners and won't show up anywhere if not used
(and if used the compiler will insert calls to strncpyto<lower|upper>
instead).

Regards,
-Markus

Re: [PATCH v2 0/7] lib: string: add functions to case-convert strings

[Alexandre Courbot]

On Wed, Jul 6, 2016 at 7:56 AM, Joe Perches <joe@perches.com> wrote:
> On Tue, 2016-07-05 at 15:36 -0700, Markus Mayer wrote:
>> On 5 July 2016 at 15:14, Joe Perches <joe@perches.com> wrote:
>> > On Tue, 2016-07-05 at 13:47 -0700, Markus Mayer wrote:
>> > > This series introduces a family of generic string case conversion
>> > > functions. This kind of functionality is needed in several places in
>> > > the kernel. Right now, everybody seems to be implementing their own
>> > > copy of this functionality.
>> > >
>> > > Based on the discussion of the previous version of this series[1] and
>> > > the use cases found in the kernel, it does look like having several
>> > > flavours of case conversion functions is beneficial. The use cases fall
>> > > into three categories:
>> > >     - copying a string and converting the case while specifying a
>> > >       maximum length to mimic strncpy()
>> > >     - copying a string and converting the case without specifying a
>> > >       length to mimic strcpy()
>> > >     - converting the case of a string in-place (i.e. modifying the
>> > >       string that was passed in)
>> > >
>> > > Consequently, I am proposing these new functions:
>> > >     char *strncpytoupper(char *dst, const char *src, size_t len);
>> > >     char *strncpytolower(char *dst, const char *src, size_t len);
>> > >     char *strcpytoupper(char *dst, const char *src);
>> > >     char *strcpytolower(char *dst, const char *src);
>> > >     char *strtoupper(char *s);
>> > >     char *strtolower(char *s);
>> > I think there isn't much value in anything other
>> > than strto.
>> >
>> > Using str[n]cpy followed by strto is
>> > pretty obvious and rarely used anyway.
>> First time around, folks were proposing the "copy" variants when I
>> submitted just strtolower() by itself[1]. They just asked for source
>> and destination parameters to strtolower(), but looking at the use
>> cases that wouldn't have worked so well. Hence it evolved into these 6
>> functions.
>>
>> Here's a breakdown of how the functions are being used (patches 2-7),
>> see also [2]:
>>
>> Patch 2: strncpytolower()
>> Patch 3: strtolower()
>> Patch 4: strncpytolower() and strtolower()
>> Patch 5: strtolower()
>> Patch 6: strcpytoupper()
>> Patch 7: strcpytoupper()
>>
>> So it does look like the copy + change case variant is more frequently
>> used than just strto.
>
> Are these functions useful?  <shrug> Not to me, not so much.
>
> None of the functions would have the strcpy performance of
> the arch / asm
> versions of strcpy and the savings in overall
> code isn't significant (or
> measured?).
>
> Of course none of the uses are runtime performance important.

I tend to agree. strcpy is better left to architecture-specific code
when it exists. Then doing a strcpy() followed by strtolower() is not
exactly unintuitive. An explosion of closely related function is
certainly more confusing to me.

I'd just keep strtolower()/strtoupper() because they are commonly done
operations and we can probably save some space by having a unique
implementation. But going beyond that is overthinking the problem
IMHO.
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH v2 0/7] lib: string: add functions to case-convert strings

[Alexandre Courbot]

On Wed, Jul 6, 2016 at 7:56 AM, Joe Perches <joe@perches.com> wrote:
> On Tue, 2016-07-05 at 15:36 -0700, Markus Mayer wrote:
>> On 5 July 2016 at 15:14, Joe Perches <joe@perches.com> wrote:
>> > On Tue, 2016-07-05 at 13:47 -0700, Markus Mayer wrote:
>> > > This series introduces a family of generic string case conversion
>> > > functions. This kind of functionality is needed in several places in
>> > > the kernel. Right now, everybody seems to be implementing their own
>> > > copy of this functionality.
>> > >
>> > > Based on the discussion of the previous version of this series[1] and
>> > > the use cases found in the kernel, it does look like having several
>> > > flavours of case conversion functions is beneficial. The use cases fall
>> > > into three categories:
>> > >     - copying a string and converting the case while specifying a
>> > >       maximum length to mimic strncpy()
>> > >     - copying a string and converting the case without specifying a
>> > >       length to mimic strcpy()
>> > >     - converting the case of a string in-place (i.e. modifying the
>> > >       string that was passed in)
>> > >
>> > > Consequently, I am proposing these new functions:
>> > >     char *strncpytoupper(char *dst, const char *src, size_t len);
>> > >     char *strncpytolower(char *dst, const char *src, size_t len);
>> > >     char *strcpytoupper(char *dst, const char *src);
>> > >     char *strcpytolower(char *dst, const char *src);
>> > >     char *strtoupper(char *s);
>> > >     char *strtolower(char *s);
>> > I think there isn't much value in anything other
>> > than strto.
>> >
>> > Using str[n]cpy followed by strto is
>> > pretty obvious and rarely used anyway.
>> First time around, folks were proposing the "copy" variants when I
>> submitted just strtolower() by itself[1]. They just asked for source
>> and destination parameters to strtolower(), but looking at the use
>> cases that wouldn't have worked so well. Hence it evolved into these 6
>> functions.
>>
>> Here's a breakdown of how the functions are being used (patches 2-7),
>> see also [2]:
>>
>> Patch 2: strncpytolower()
>> Patch 3: strtolower()
>> Patch 4: strncpytolower() and strtolower()
>> Patch 5: strtolower()
>> Patch 6: strcpytoupper()
>> Patch 7: strcpytoupper()
>>
>> So it does look like the copy + change case variant is more frequently
>> used than just strto.
>
> Are these functions useful?  <shrug> Not to me, not so much.
>
> None of the functions would have the strcpy performance of
> the arch / asm
> versions of strcpy and the savings in overall
> code isn't significant (or
> measured?).
>
> Of course none of the uses are runtime performance important.

I tend to agree. strcpy is better left to architecture-specific code
when it exists. Then doing a strcpy() followed by strtolower() is not
exactly unintuitive. An explosion of closely related function is
certainly more confusing to me.

I'd just keep strtolower()/strtoupper() because they are commonly done
operations and we can probably save some space by having a unique
implementation. But going beyond that is overthinking the problem
IMHO.

Re: [PATCH v2 1/7] lib: string: add functions to case-convert strings

[Eric Engestrom]

On Tue, Jul 05, 2016 at 01:47:05PM -0700, Markus Mayer wrote:
> All functions return a pointer to the terminating '\0' character in the
> modified string ("dst" or "s", respectively).

I think this is going to be confusing. From the man:

	The strcpy() and strncpy() functions return a pointer to the
	destination string dest.

I think it would be better to keep the same behaviour, especially since
you used the same name for your functions (which I think is sensible),
not to mention you don't use this return value in any of your calls.
(IMHO strcpy() shouldn't have had a return value and neither should your
functions, but since it does, yours should match.)

Cheers,
  Eric
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/7] lib: string: add functions to case-convert strings

[Eric Engestrom]

On Tue, Jul 05, 2016 at 01:47:05PM -0700, Markus Mayer wrote:
> All functions return a pointer to the terminating '\0' character in the
> modified string ("dst" or "s", respectively).

I think this is going to be confusing. From the man:

	The strcpy() and strncpy() functions return a pointer to the
	destination string dest.

I think it would be better to keep the same behaviour, especially since
you used the same name for your functions (which I think is sensible),
not to mention you don't use this return value in any of your calls.
(IMHO strcpy() shouldn't have had a return value and neither should your
functions, but since it does, yours should match.)

Cheers,
  Eric

Re: [PATCH v2 1/7] lib: string: add functions to case-convert strings

[Rasmus Villemoes]

On Tue, Jul 05 2016, Markus Mayer <mmayer@broadcom.com> wrote:

> Add a collection of generic functions to convert strings to lowercase
> or uppercase.
>
> Changing the case of a string (with or without copying it first) seems
> to be a recurring requirement in the kernel that is currently being
> solved by several duplicated implementations doing the same thing. This
> change aims at reducing this code duplication.
>
> +/**
> + * strncpytoupper - Copy a length-limited string and convert to uppercase.
> + * @dst: The buffer to store the result.
> + * @src: The string to convert to uppercase.
> + * @len: Maximum string length. May be 0 to set no limit.
> + *
> + * Returns pointer to terminating '\0' in @dst.
> + */
> +char *strncpytoupper(char *dst, const char *src, size_t len)
> +{
> +	size_t i;
> +
> +	for (i = 0; src[i] != '\0' && (i < len || !len); i++)
> +		dst[i] = toupper(src[i]);
> +	if (i < len || !len)
> +		dst[i] = '\0';
> +
> +	return dst + i;
> +}

Hm, this seems to copy the insane semantics from strncpy of not
guaranteeing '\0'-termination.

Why use 0 as a sentinel, when (size_t)-1 == SIZE_MAX would work just as
well and require a little less code (no || !len)?

I regret suggesting this return semantics and now agree that void would
be better, especially since there doesn't seem to be anyone who can
use this (or any other) return value. How about

if (!len)
   return;

for (i = 0; i < len && src[i]; ++i)
  dst[i] = toupper(src[i]);
dst[i < len ? i : i-1] = '\0';

(I think you must do i < len before testing src[i], since the len
parameter should be an upper bound on the number of bytes to access in
both src and dst).

Rasmus
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH v2 1/7] lib: string: add functions to case-convert strings

[Rasmus Villemoes]

On Tue, Jul 05 2016, Markus Mayer <mmayer@broadcom.com> wrote:

> Add a collection of generic functions to convert strings to lowercase
> or uppercase.
>
> Changing the case of a string (with or without copying it first) seems
> to be a recurring requirement in the kernel that is currently being
> solved by several duplicated implementations doing the same thing. This
> change aims at reducing this code duplication.
>
> +/**
> + * strncpytoupper - Copy a length-limited string and convert to uppercase.
> + * @dst: The buffer to store the result.
> + * @src: The string to convert to uppercase.
> + * @len: Maximum string length. May be 0 to set no limit.
> + *
> + * Returns pointer to terminating '\0' in @dst.
> + */
> +char *strncpytoupper(char *dst, const char *src, size_t len)
> +{
> +	size_t i;
> +
> +	for (i = 0; src[i] != '\0' && (i < len || !len); i++)
> +		dst[i] = toupper(src[i]);
> +	if (i < len || !len)
> +		dst[i] = '\0';
> +
> +	return dst + i;
> +}

Hm, this seems to copy the insane semantics from strncpy of not
guaranteeing '\0'-termination.

Why use 0 as a sentinel, when (size_t)-1 == SIZE_MAX would work just as
well and require a little less code (no || !len)?

I regret suggesting this return semantics and now agree that void would
be better, especially since there doesn't seem to be anyone who can
use this (or any other) return value. How about

if (!len)
   return;

for (i = 0; i < len && src[i]; ++i)
  dst[i] = toupper(src[i]);
dst[i < len ? i : i-1] = '\0';

(I think you must do i < len before testing src[i], since the len
parameter should be an upper bound on the number of bytes to access in
both src and dst).

Rasmus

Re: [PATCH v2 1/7] lib: string: add functions to case-convert strings

[Markus Mayer]

On 7 July 2016 at 17:19, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
> On Tue, Jul 05 2016, Markus Mayer <mmayer@broadcom.com> wrote:
>
>> +/**
>> + * strncpytoupper - Copy a length-limited string and convert to uppercase.
>> + * @dst: The buffer to store the result.
>> + * @src: The string to convert to uppercase.
>> + * @len: Maximum string length. May be 0 to set no limit.
>> + *
>> + * Returns pointer to terminating '\0' in @dst.
>> + */
>> +char *strncpytoupper(char *dst, const char *src, size_t len)
>> +{
>> +     size_t i;
>> +
>> +     for (i = 0; src[i] != '\0' && (i < len || !len); i++)
>> +             dst[i] = toupper(src[i]);
>> +     if (i < len || !len)
>> +             dst[i] = '\0';
>> +
>> +     return dst + i;
>> +}
>
> Hm, this seems to copy the insane semantics from strncpy of not
> guaranteeing '\0'-termination.

Yeah. I've been tossing that one around a bit. The reason I did it
this way in the end is due to the use cases I found. strncpy() is
being used there and I was a little wary to make too many changes all
at once.

But I understand your point. I'll look at it again and see what
changes might be required in the code that used strncpy() before.

> Why use 0 as a sentinel, when (size_t)-1 == SIZE_MAX would work just as
> well and require a little less code (no || !len)?

I'll change that.

> I regret suggesting this return semantics and now agree that void would
> be better, especially since there doesn't seem to be anyone who can
> use this (or any other) return value. How about
>
> if (!len)
>    return;
>
> for (i = 0; i < len && src[i]; ++i)
>   dst[i] = toupper(src[i]);
> dst[i < len ? i : i-1] = '\0';

This makes sense.

> (I think you must do i < len before testing src[i], since the len
> parameter should be an upper bound on the number of bytes to access in
> both src and dst).
>
> Rasmus

Thanks,
-Markus
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH v2 1/7] lib: string: add functions to case-convert strings

[Markus Mayer]

On 7 July 2016 at 17:19, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
> On Tue, Jul 05 2016, Markus Mayer <mmayer@broadcom.com> wrote:
>
>> +/**
>> + * strncpytoupper - Copy a length-limited string and convert to uppercase.
>> + * @dst: The buffer to store the result.
>> + * @src: The string to convert to uppercase.
>> + * @len: Maximum string length. May be 0 to set no limit.
>> + *
>> + * Returns pointer to terminating '\0' in @dst.
>> + */
>> +char *strncpytoupper(char *dst, const char *src, size_t len)
>> +{
>> +     size_t i;
>> +
>> +     for (i = 0; src[i] != '\0' && (i < len || !len); i++)
>> +             dst[i] = toupper(src[i]);
>> +     if (i < len || !len)
>> +             dst[i] = '\0';
>> +
>> +     return dst + i;
>> +}
>
> Hm, this seems to copy the insane semantics from strncpy of not
> guaranteeing '\0'-termination.

Yeah. I've been tossing that one around a bit. The reason I did it
this way in the end is due to the use cases I found. strncpy() is
being used there and I was a little wary to make too many changes all
at once.

But I understand your point. I'll look at it again and see what
changes might be required in the code that used strncpy() before.

> Why use 0 as a sentinel, when (size_t)-1 == SIZE_MAX would work just as
> well and require a little less code (no || !len)?

I'll change that.

> I regret suggesting this return semantics and now agree that void would
> be better, especially since there doesn't seem to be anyone who can
> use this (or any other) return value. How about
>
> if (!len)
>    return;
>
> for (i = 0; i < len && src[i]; ++i)
>   dst[i] = toupper(src[i]);
> dst[i < len ? i : i-1] = '\0';

This makes sense.

> (I think you must do i < len before testing src[i], since the len
> parameter should be an upper bound on the number of bytes to access in
> both src and dst).
>
> Rasmus

Thanks,
-Markus