Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1434910 > unrolled thread

[PATCH 0/6] lib: string: add function strtolower()

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/6] lib: string: add function strtolower() Markus Mayer <mmayer@broadcom.com> - 2016-07-01 02:00 +0200
    [PATCH 4/6] ACPI / device_sysfs: make use of new strtolower() function Markus Mayer <mmayer@broadcom.com> - 2016-07-01 02:00 +0200
      Re: [PATCH 4/6] ACPI / device_sysfs: make use of new strtolower() function "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-07-01 22:50 +0200
    [PATCH 2/6] drm/nouveau/core: make use of new strtolower() function Markus Mayer <mmayer@broadcom.com> - 2016-07-01 02:00 +0200
      Re: [Nouveau] [PATCH 2/6] drm/nouveau/core: make use of new  strtolower() function Alexandre Courbot <gnurou@gmail.com> - 2016-07-02 03:20 +0200
        Re: [Nouveau] [PATCH 2/6] drm/nouveau/core: make use of new  strtolower() function Markus Mayer <markus.mayer@broadcom.com> - 2016-07-02 17:30 +0200
          Re: [Nouveau] [PATCH 2/6] drm/nouveau/core: make use of new  strtolower() function Alexandre Courbot <gnurou@gmail.com> - 2016-07-04 03:40 +0200
            Re: [Nouveau] [PATCH 2/6] drm/nouveau/core: make use of new  strtolower() function Alexandre Courbot <gnurou@gmail.com> - 2016-07-04 05:50 +0200
    [PATCH 3/6] ACPICA: make use of new strtolower() function Markus Mayer <mmayer@broadcom.com> - 2016-07-01 02:00 +0200
      RE: [PATCH 3/6] ACPICA: make use of new strtolower() function "Moore, Robert" <robert.moore@intel.com> - 2016-07-01 03:20 +0200
        Re: [PATCH 3/6] ACPICA: make use of new strtolower() function Markus Mayer <markus.mayer@broadcom.com> - 2016-07-01 06:20 +0200
          RE: [PATCH 3/6] ACPICA: make use of new strtolower() function "Moore, Robert" <robert.moore@intel.com> - 2016-07-01 06:40 +0200
    [PATCH 5/6] staging: speakup: replace spk_strlwr() with strtolower() Markus Mayer <mmayer@broadcom.com> - 2016-07-01 02:00 +0200
      Re: [PATCH 5/6] staging: speakup: replace spk_strlwr() with  strtolower() Samuel Thibault <samuel.thibault@ens-lyon.org> - 2016-07-01 02:40 +0200
    [PATCH 1/6] lib: string: add function strtolower() Markus Mayer <mmayer@broadcom.com> - 2016-07-01 02:00 +0200
      Re: [PATCH 1/6] lib: string: add function strtolower() Jani Nikula <jani.nikula@linux.intel.com> - 2016-07-01 13:00 +0200
        Re: [PATCH 1/6] lib: string: add function strtolower() Markus Mayer <markus.mayer@broadcom.com> - 2016-07-01 19:20 +0200
          Re: [PATCH 1/6] lib: string: add function strtolower() Jani Nikula <jani.nikula@linux.intel.com> - 2016-07-01 19:40 +0200
      Re: [PATCH 1/6] lib: string: add function strtolower() Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2016-07-01 23:10 +0200
        Re: [PATCH 1/6] lib: string: add function strtolower() Markus Mayer <markus.mayer@broadcom.com> - 2016-07-04 22:20 +0200

#1434910 — [PATCH 0/6] lib: string: add function strtolower()

This series introduces a new generic function strtolower(), which
converts strings to lowercase in-place, overwriting the original
string. This kind of functionality is needed in several places in the
kernel. Right now, everybody seems to be implementing their own copy of
this function. So, we replace several custom "strtolower"
implementations with this new library function.

Another driver that also makes use of this function 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.

This series is based on v4.7-rc5.

Markus Mayer (6):
  lib: string: add function strtolower()
  drm/nouveau/core: make use of new strtolower() function
  ACPICA: make use of new strtolower() function
  ACPI / device_sysfs: make use of new strtolower() function
  staging: speakup: replace spk_strlwr() with strtolower()
  iscsi-target: replace iscsi_initiatorname_tolower() with strtolower()

 drivers/acpi/acpica/utnonansi.c              | 13 +------------
 drivers/acpi/device_sysfs.c                  |  4 +---
 drivers/gpu/drm/nouveau/nvkm/core/firmware.c |  7 +------
 drivers/staging/speakup/kobjects.c           |  2 +-
 drivers/staging/speakup/main.c               |  2 +-
 drivers/staging/speakup/speakup.h            |  1 -
 drivers/staging/speakup/varhandlers.c        | 12 ------------
 drivers/target/iscsi/iscsi_target_nego.c     | 17 +----------------
 include/linux/string.h                       |  1 +
 lib/string.c                                 | 14 ++++++++++++++
 10 files changed, 21 insertions(+), 52 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]

#1434911 — [PATCH 4/6] ACPI / device_sysfs: make use of new strtolower() function

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

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 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

[toc] | [prev] | [next] | [standalone]

#1435617 — Re: [PATCH 4/6] ACPI / device_sysfs: make use of new strtolower() function

On Thursday, June 30, 2016 04:50:10 PM Markus Mayer wrote:
> Call strtolower() rather than walking the string explicitly to convert
> it to lowercase.
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>

ACK

> ---
>  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);
> 

[toc] | [prev] | [next] | [standalone]

#1434912 — [PATCH 2/6] drm/nouveau/core: make use of new strtolower() function

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

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

diff --git a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
index 34ecd4a..c50594c 100644
--- a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
+++ b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
@@ -36,16 +36,11 @@ 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));
 	cname[sizeof(cname) - 1] = '\0';
-	i = strlen(cname);
-	while (i) {
-		--i;
-		cname[i] = tolower(cname[i]);
-	}
+	strtolower(cname);
 
 	snprintf(f, sizeof(f), "nvidia/%s/%s.bin", cname, fwname);
 	return request_firmware(fw, f, device->dev);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1435707 — Re: [Nouveau] [PATCH 2/6] drm/nouveau/core: make use of new strtolower() function

On Fri, Jul 1, 2016 at 8:50 AM, Markus Mayer <mmayer@broadcom.com> wrote:
> Call strtolower() rather than walking the string explicitly to convert
> it to lowercase.
>
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> ---
>  drivers/gpu/drm/nouveau/nvkm/core/firmware.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
> index 34ecd4a..c50594c 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
> @@ -36,16 +36,11 @@ 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));
>         cname[sizeof(cname) - 1] = '\0';
> -       i = strlen(cname);
> -       while (i) {
> -               --i;
> -               cname[i] = tolower(cname[i]);
> -       }
> +       strtolower(cname);

This function doesn't seem to exist as of next-20160701, where have
you found it?

[toc] | [prev] | [next] | [standalone]

#1435833 — Re: [Nouveau] [PATCH 2/6] drm/nouveau/core: make use of new strtolower() function

On 1 July 2016 at 18:18, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Fri, Jul 1, 2016 at 8:50 AM, Markus Mayer <mmayer@broadcom.com> wrote:
>> Call strtolower() rather than walking the string explicitly to convert
>> it to lowercase.
>>
>> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>> ---
>>  drivers/gpu/drm/nouveau/nvkm/core/firmware.c | 7 +------
>>  1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
>> index 34ecd4a..c50594c 100644
>> --- a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
>> +++ b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
>> @@ -36,16 +36,11 @@ 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));
>>         cname[sizeof(cname) - 1] = '\0';
>> -       i = strlen(cname);
>> -       while (i) {
>> -               --i;
>> -               cname[i] = tolower(cname[i]);
>> -       }
>> +       strtolower(cname);
>
> This function doesn't seem to exist as of next-20160701, where have
> you found it?

I didn't find it. I wrote it, because it didn't exist and I needed it.
See: https://lkml.org/lkml/2016/6/30/727 and
https://lkml.org/lkml/2016/6/30/733 (cover letter and first patch in
series).

[toc] | [prev] | [next] | [standalone]

#1436105 — Re: [Nouveau] [PATCH 2/6] drm/nouveau/core: make use of new strtolower() function

On Sun, Jul 3, 2016 at 12:21 AM, Markus Mayer <markus.mayer@broadcom.com> wrote:
> On 1 July 2016 at 18:18, Alexandre Courbot <gnurou@gmail.com> wrote:
>> On Fri, Jul 1, 2016 at 8:50 AM, Markus Mayer <mmayer@broadcom.com> wrote:
>>> Call strtolower() rather than walking the string explicitly to convert
>>> it to lowercase.
>>>
>>> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>>> ---
>>>  drivers/gpu/drm/nouveau/nvkm/core/firmware.c | 7 +------
>>>  1 file changed, 1 insertion(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
>>> index 34ecd4a..c50594c 100644
>>> --- a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
>>> +++ b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
>>> @@ -36,16 +36,11 @@ 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));
>>>         cname[sizeof(cname) - 1] = '\0';
>>> -       i = strlen(cname);
>>> -       while (i) {
>>> -               --i;
>>> -               cname[i] = tolower(cname[i]);
>>> -       }
>>> +       strtolower(cname);
>>
>> This function doesn't seem to exist as of next-20160701, where have
>> you found it?
>
> I didn't find it. I wrote it, because it didn't exist and I needed it.
> See: https://lkml.org/lkml/2016/6/30/727 and
> https://lkml.org/lkml/2016/6/30/733 (cover letter and first patch in
> series).

Ah, right - would have been easier to understand if you had sent the
whole series (or at least patches 0 to 2) to us as well. Please do
that for the next version.

[toc] | [prev] | [next] | [standalone]

#1436279 — Re: [Nouveau] [PATCH 2/6] drm/nouveau/core: make use of new strtolower() function

On Mon, Jul 4, 2016 at 10:37 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Sun, Jul 3, 2016 at 12:21 AM, Markus Mayer <markus.mayer@broadcom.com> wrote:
>> On 1 July 2016 at 18:18, Alexandre Courbot <gnurou@gmail.com> wrote:
>>> On Fri, Jul 1, 2016 at 8:50 AM, Markus Mayer <mmayer@broadcom.com> wrote:
>>>> Call strtolower() rather than walking the string explicitly to convert
>>>> it to lowercase.
>>>>
>>>> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>>>> ---
>>>>  drivers/gpu/drm/nouveau/nvkm/core/firmware.c | 7 +------
>>>>  1 file changed, 1 insertion(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
>>>> index 34ecd4a..c50594c 100644
>>>> --- a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
>>>> +++ b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
>>>> @@ -36,16 +36,11 @@ 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));
>>>>         cname[sizeof(cname) - 1] = '\0';
>>>> -       i = strlen(cname);
>>>> -       while (i) {
>>>> -               --i;
>>>> -               cname[i] = tolower(cname[i]);
>>>> -       }
>>>> +       strtolower(cname);
>>>
>>> This function doesn't seem to exist as of next-20160701, where have
>>> you found it?
>>
>> I didn't find it. I wrote it, because it didn't exist and I needed it.
>> See: https://lkml.org/lkml/2016/6/30/727 and
>> https://lkml.org/lkml/2016/6/30/733 (cover letter and first patch in
>> series).
>
> Ah, right - would have been easier to understand if you had sent the
> whole series (or at least patches 0 to 2) to us as well. Please do
> that for the next version.

... found patches 0 and 1 in my spam folder, for some weird reason.
Apologies for jumping to conclusions.

[toc] | [prev] | [next] | [standalone]

#1434913 — [PATCH 3/6] ACPICA: make use of new strtolower() function

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

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

*** Please note that there don't seem to be any callers of acpi_ut_strlwr().
*** It may be possible to remove the function altogether.

 drivers/acpi/acpica/utnonansi.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/acpi/acpica/utnonansi.c b/drivers/acpi/acpica/utnonansi.c
index 3465fe2..b6e11dc 100644
--- a/drivers/acpi/acpica/utnonansi.c
+++ b/drivers/acpi/acpica/utnonansi.c
@@ -64,19 +64,8 @@ ACPI_MODULE_NAME("utnonansi")
  ******************************************************************************/
 void acpi_ut_strlwr(char *src_string)
 {
-	char *string;
-
 	ACPI_FUNCTION_ENTRY();
-
-	if (!src_string) {
-		return;
-	}
-
-	/* Walk entire string, lowercasing the letters */
-
-	for (string = src_string; *string; string++) {
-		*string = (char)tolower((int)*string);
-	}
+	strtolower(src_string);
 }
 
 /*******************************************************************************
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1434942 — RE: [PATCH 3/6] ACPICA: make use of new strtolower() function

Where is "strtolower" implemented?


> -----Original Message-----
> From: Markus Mayer [mailto:mmayer@broadcom.com]
> Sent: Thursday, June 30, 2016 4:50 PM
> To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J; Len Brown
> Cc: Markus Mayer; linux-acpi@vger.kernel.org; devel@acpica.org; linux-
> kernel@vger.kernel.org
> Subject: [PATCH 3/6] ACPICA: make use of new strtolower() function
> 
> Call strtolower() rather than walking the string explicitly to convert it
> to lowercase.
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> ---
> 
> *** Please note that there don't seem to be any callers of
> acpi_ut_strlwr().
> *** It may be possible to remove the function altogether.
> 
>  drivers/acpi/acpica/utnonansi.c | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/drivers/acpi/acpica/utnonansi.c
> b/drivers/acpi/acpica/utnonansi.c index 3465fe2..b6e11dc 100644
> --- a/drivers/acpi/acpica/utnonansi.c
> +++ b/drivers/acpi/acpica/utnonansi.c
> @@ -64,19 +64,8 @@ ACPI_MODULE_NAME("utnonansi")
> 
> **************************************************************************
> ****/
>  void acpi_ut_strlwr(char *src_string)
>  {
> -	char *string;
> -
>  	ACPI_FUNCTION_ENTRY();
> -
> -	if (!src_string) {
> -		return;
> -	}
> -
> -	/* Walk entire string, lowercasing the letters */
> -
> -	for (string = src_string; *string; string++) {
> -		*string = (char)tolower((int)*string);
> -	}
> +	strtolower(src_string);
>  }
> 
> 
> /*************************************************************************
> ******
> --
> 2.7.4

[toc] | [prev] | [next] | [standalone]

#1435003 — Re: [PATCH 3/6] ACPICA: make use of new strtolower() function

On 30 June 2016 at 19:59, Moore, Robert <robert.moore@intel.com> wrote:
> This is linux-specific code, ACPICA is os-independent. So we cannot accept
> such patch.

Understood. I wasn't aware that this was shared code.

> From: Markus Mayer [mailto:markus.mayer@broadcom.com]
> Sent: Thursday, June 30, 2016 7:50 PM
> To: Moore, Robert
> Cc: Zheng, Lv; Wysocki, Rafael J; Len Brown; linux-acpi@vger.kernel.org;
> devel@acpica.org; linux-kernel@vger.kernel.org; Box, David E
> Subject: Re: [PATCH 3/6] ACPICA: make use of new strtolower() function
>
> On Thursday, 30 June 2016, Moore, Robert <robert.moore@intel.com> wrote:
>
> Where is "strtolower" implemented?
>
> First patch of the series:
>
> https://lkml.org/lkml/2016/6/30/733
>
>> -----Original Message-----
>> From: Markus Mayer [mailto:mmayer@broadcom.com]
>> Sent: Thursday, June 30, 2016 4:50 PM
>> To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J; Len Brown
>> Cc: Markus Mayer; linux-acpi@vger.kernel.org; devel@acpica.org; linux-
>> kernel@vger.kernel.org
>> Subject: [PATCH 3/6] ACPICA: make use of new strtolower() function
>>
>> Call strtolower() rather than walking the string explicitly to convert it
>> to lowercase.
>>
>> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>> ---
>>
>> *** Please note that there don't seem to be any callers of
>> acpi_ut_strlwr().
>> *** It may be possible to remove the function altogether.
>>
>>  drivers/acpi/acpica/utnonansi.c | 13 +------------
>>  1 file changed, 1 insertion(+), 12 deletions(-)
>>
>> diff --git a/drivers/acpi/acpica/utnonansi.c
>> b/drivers/acpi/acpica/utnonansi.c index 3465fe2..b6e11dc 100644
>> --- a/drivers/acpi/acpica/utnonansi.c
>> +++ b/drivers/acpi/acpica/utnonansi.c
>> @@ -64,19 +64,8 @@ ACPI_MODULE_NAME("utnonansi")
>>
>> **************************************************************************
>> ****/
>>  void acpi_ut_strlwr(char *src_string)
>>  {
>> -     char *string;
>> -
>>       ACPI_FUNCTION_ENTRY();
>> -
>> -     if (!src_string) {
>> -             return;
>> -     }
>> -
>> -     /* Walk entire string, lowercasing the letters */
>> -
>> -     for (string = src_string; *string; string++) {
>> -             *string = (char)tolower((int)*string);
>> -     }
>> +     strtolower(src_string);
>>  }
>>
>>
>> /*************************************************************************
>> ******
>> --
>> 2.7.4

[toc] | [prev] | [next] | [standalone]

#1435009 — RE: [PATCH 3/6] ACPICA: make use of new strtolower() function

> -----Original Message-----
> From: Markus Mayer [mailto:markus.mayer@broadcom.com]
> Sent: Thursday, June 30, 2016 9:13 PM
> To: Moore, Robert
> Cc: Zheng, Lv; Wysocki, Rafael J; Len Brown; linux-acpi@vger.kernel.org;
> devel@acpica.org; linux-kernel@vger.kernel.org; Box, David E
> Subject: Re: [PATCH 3/6] ACPICA: make use of new strtolower() function
> 
> On 30 June 2016 at 19:59, Moore, Robert <robert.moore@intel.com> wrote:
> > This is linux-specific code, ACPICA is os-independent. So we cannot
> > accept such patch.
> 
> Understood. I wasn't aware that this was shared code.

Ok. Glad to take fixes and optimizations, however.
Bob



> 
> > From: Markus Mayer [mailto:markus.mayer@broadcom.com]
> > Sent: Thursday, June 30, 2016 7:50 PM
> > To: Moore, Robert
> > Cc: Zheng, Lv; Wysocki, Rafael J; Len Brown;
> > linux-acpi@vger.kernel.org; devel@acpica.org;
> > linux-kernel@vger.kernel.org; Box, David E
> > Subject: Re: [PATCH 3/6] ACPICA: make use of new strtolower() function
> >
> > On Thursday, 30 June 2016, Moore, Robert <robert.moore@intel.com> wrote:
> >
> > Where is "strtolower" implemented?
> >
> > First patch of the series:
> >
> > https://lkml.org/lkml/2016/6/30/733
> >
> >> -----Original Message-----
> >> From: Markus Mayer [mailto:mmayer@broadcom.com]
> >> Sent: Thursday, June 30, 2016 4:50 PM
> >> To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J; Len Brown
> >> Cc: Markus Mayer; linux-acpi@vger.kernel.org; devel@acpica.org;
> >> linux- kernel@vger.kernel.org
> >> Subject: [PATCH 3/6] ACPICA: make use of new strtolower() function
> >>
> >> Call strtolower() rather than walking the string explicitly to
> >> convert it to lowercase.
> >>
> >> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> >> ---
> >>
> >> *** Please note that there don't seem to be any callers of
> >> acpi_ut_strlwr().
> >> *** It may be possible to remove the function altogether.
> >>
> >>  drivers/acpi/acpica/utnonansi.c | 13 +------------
> >>  1 file changed, 1 insertion(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/acpi/acpica/utnonansi.c
> >> b/drivers/acpi/acpica/utnonansi.c index 3465fe2..b6e11dc 100644
> >> --- a/drivers/acpi/acpica/utnonansi.c
> >> +++ b/drivers/acpi/acpica/utnonansi.c
> >> @@ -64,19 +64,8 @@ ACPI_MODULE_NAME("utnonansi")
> >>
> >> *********************************************************************
> >> *****
> >> ****/
> >>  void acpi_ut_strlwr(char *src_string)  {
> >> -     char *string;
> >> -
> >>       ACPI_FUNCTION_ENTRY();
> >> -
> >> -     if (!src_string) {
> >> -             return;
> >> -     }
> >> -
> >> -     /* Walk entire string, lowercasing the letters */
> >> -
> >> -     for (string = src_string; *string; string++) {
> >> -             *string = (char)tolower((int)*string);
> >> -     }
> >> +     strtolower(src_string);
> >>  }
> >>
> >>
> >> /********************************************************************
> >> *****
> >> ******
> >> --
> >> 2.7.4

[toc] | [prev] | [next] | [standalone]

#1434914 — [PATCH 5/6] staging: speakup: replace spk_strlwr() with strtolower()

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

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 drivers/staging/speakup/kobjects.c    |  2 +-
 drivers/staging/speakup/main.c        |  2 +-
 drivers/staging/speakup/speakup.h     |  1 -
 drivers/staging/speakup/varhandlers.c | 12 ------------
 4 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/speakup/kobjects.c b/drivers/staging/speakup/kobjects.c
index 528cbdc..009c0d2 100644
--- a/drivers/staging/speakup/kobjects.c
+++ b/drivers/staging/speakup/kobjects.c
@@ -391,7 +391,7 @@ static ssize_t synth_store(struct kobject *kobj, struct kobj_attribute *attr,
 	if (new_synth_name[len - 1] == '\n')
 		len--;
 	new_synth_name[len] = '\0';
-	spk_strlwr(new_synth_name);
+	strtolower(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..3f3f254 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -2314,7 +2314,7 @@ 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);
+	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

[toc] | [prev] | [next] | [standalone]

#1434931 — Re: [PATCH 5/6] staging: speakup: replace spk_strlwr() with strtolower()

Markus Mayer, on Thu 30 Jun 2016 16:50:11 -0700, wrote:
> After introducing generic strtolower(), spk_strlwr() is no longer
> needed.
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>

Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> ---
>  drivers/staging/speakup/kobjects.c    |  2 +-
>  drivers/staging/speakup/main.c        |  2 +-
>  drivers/staging/speakup/speakup.h     |  1 -
>  drivers/staging/speakup/varhandlers.c | 12 ------------
>  4 files changed, 2 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/staging/speakup/kobjects.c b/drivers/staging/speakup/kobjects.c
> index 528cbdc..009c0d2 100644
> --- a/drivers/staging/speakup/kobjects.c
> +++ b/drivers/staging/speakup/kobjects.c
> @@ -391,7 +391,7 @@ static ssize_t synth_store(struct kobject *kobj, struct kobj_attribute *attr,
>  	if (new_synth_name[len - 1] == '\n')
>  		len--;
>  	new_synth_name[len] = '\0';
> -	spk_strlwr(new_synth_name);
> +	strtolower(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..3f3f254 100644
> --- a/drivers/staging/speakup/main.c
> +++ b/drivers/staging/speakup/main.c
> @@ -2314,7 +2314,7 @@ 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);
> +	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
> 

-- 
Samuel
<D> m'enfin, le 5 juillet, le mec vient visiter le labo...
* D a marque d'une croix rouge le 5 juillet sur son agenda
<y> niarc niarc niarc
<D> cet homme va souffrir
<B> c'est donc le 5 juillet qu'il meurt d'un accident de la route écrasé par un truck muni d'un pare buffle
 -+- #ens-mim - repaire de terroristes -+-

[toc] | [prev] | [next] | [standalone]

#1434915 — [PATCH 1/6] lib: string: add function strtolower()

Add a function called strtolower() to convert strings to lower case
in-place, overwriting the original string.

This seems to be a recurring requirement in the kernel that is
currently being solved by several duplicated implementations doing the
same thing.

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

diff --git a/include/linux/string.h b/include/linux/string.h
index 26b6f6a..aad605e 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -116,6 +116,7 @@ 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 *strtolower(char *s);
 
 extern void kfree_const(const void *x);
 
diff --git a/lib/string.c b/lib/string.c
index ed83562..6e3b560 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -952,3 +952,17 @@ char *strreplace(char *s, char old, char new)
 	return s;
 }
 EXPORT_SYMBOL(strreplace);
+
+char *strtolower(char *s)
+{
+	char *p;
+
+        if (unlikely(!s))
+                return NULL;
+
+	for (p = s; *p; p++)
+		*p = tolower(*p);
+
+	return s;
+}
+EXPORT_SYMBOL(strtolower);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1435256 — Re: [PATCH 1/6] lib: string: add function strtolower()

On Fri, 01 Jul 2016, Markus Mayer <mmayer@broadcom.com> wrote:
> Add a function called strtolower() to convert strings to lower case
> in-place, overwriting the original string.
>
> This seems to be a recurring requirement in the kernel that is
> currently being solved by several duplicated implementations doing the
> same thing.
>
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> ---
>  include/linux/string.h |  1 +
>  lib/string.c           | 14 ++++++++++++++
>  2 files changed, 15 insertions(+)
>
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 26b6f6a..aad605e 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -116,6 +116,7 @@ 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 *strtolower(char *s);
>  
>  extern void kfree_const(const void *x);
>  
> diff --git a/lib/string.c b/lib/string.c
> index ed83562..6e3b560 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -952,3 +952,17 @@ char *strreplace(char *s, char old, char new)
>  	return s;
>  }
>  EXPORT_SYMBOL(strreplace);
> +

This needs a kernel-doc comment right here.

> +char *strtolower(char *s)
> +{
> +	char *p;
> +
> +        if (unlikely(!s))
> +                return NULL;

Using spaces for indentation? See scripts/checkpatch.pl.

> +
> +	for (p = s; *p; p++)
> +		*p = tolower(*p);
> +
> +	return s;

Why does it return a value? Could be void?

BR,
Jani.

> +}
> +EXPORT_SYMBOL(strtolower);

-- 
Jani Nikula, Intel Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1435509 — Re: [PATCH 1/6] lib: string: add function strtolower()

On 1 July 2016 at 03:52, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Fri, 01 Jul 2016, Markus Mayer <mmayer@broadcom.com> wrote:
>> Add a function called strtolower() to convert strings to lower case
>> in-place, overwriting the original string.
>>
>> This seems to be a recurring requirement in the kernel that is
>> currently being solved by several duplicated implementations doing the
>> same thing.
>>
>> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>> ---
>>  include/linux/string.h |  1 +
>>  lib/string.c           | 14 ++++++++++++++
>>  2 files changed, 15 insertions(+)
>>
>> diff --git a/include/linux/string.h b/include/linux/string.h
>> index 26b6f6a..aad605e 100644
>> --- a/include/linux/string.h
>> +++ b/include/linux/string.h
>> @@ -116,6 +116,7 @@ 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 *strtolower(char *s);
>>
>>  extern void kfree_const(const void *x);
>>
>> diff --git a/lib/string.c b/lib/string.c
>> index ed83562..6e3b560 100644
>> --- a/lib/string.c
>> +++ b/lib/string.c
>> @@ -952,3 +952,17 @@ char *strreplace(char *s, char old, char new)
>>       return s;
>>  }
>>  EXPORT_SYMBOL(strreplace);
>> +
>
> This needs a kernel-doc comment right here.

Will add it.

>> +char *strtolower(char *s)
>> +{
>> +     char *p;
>> +
>> +        if (unlikely(!s))
>> +                return NULL;
>
> Using spaces for indentation? See scripts/checkpatch.pl.

Not on purpose. Thanks for spotting it.

>> +
>> +     for (p = s; *p; p++)
>> +             *p = tolower(*p);
>> +
>> +     return s;
>
> Why does it return a value? Could be void?

It could be void, but I thought that would make the function's use
less flexible. As is, the return value is there if anybody wants it,
but it can be ignored if it is not needed. Also, it seems customary
for string functions to be returning the string that was passed in.

I'll change it to void if there are strong opinions leaning that way.
Personally, I like that it returns a char * better.

> BR,
> Jani.
>
>> +}
>> +EXPORT_SYMBOL(strtolower);
>
> --
> Jani Nikula, Intel Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1435522 — Re: [PATCH 1/6] lib: string: add function strtolower()

On Fri, 01 Jul 2016, Markus Mayer <markus.mayer@broadcom.com> wrote:
> On 1 July 2016 at 03:52, Jani Nikula <jani.nikula@linux.intel.com> wrote:
>> On Fri, 01 Jul 2016, Markus Mayer <mmayer@broadcom.com> wrote:
>>> Add a function called strtolower() to convert strings to lower case
>>> in-place, overwriting the original string.
>>>
>>> This seems to be a recurring requirement in the kernel that is
>>> currently being solved by several duplicated implementations doing the
>>> same thing.
>>>
>>> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>>> ---
>>>  include/linux/string.h |  1 +
>>>  lib/string.c           | 14 ++++++++++++++
>>>  2 files changed, 15 insertions(+)
>>>
>>> diff --git a/include/linux/string.h b/include/linux/string.h
>>> index 26b6f6a..aad605e 100644
>>> --- a/include/linux/string.h
>>> +++ b/include/linux/string.h
>>> @@ -116,6 +116,7 @@ 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 *strtolower(char *s);
>>>
>>>  extern void kfree_const(const void *x);
>>>
>>> diff --git a/lib/string.c b/lib/string.c
>>> index ed83562..6e3b560 100644
>>> --- a/lib/string.c
>>> +++ b/lib/string.c
>>> @@ -952,3 +952,17 @@ char *strreplace(char *s, char old, char new)
>>>       return s;
>>>  }
>>>  EXPORT_SYMBOL(strreplace);
>>> +
>>
>> This needs a kernel-doc comment right here.
>
> Will add it.
>
>>> +char *strtolower(char *s)
>>> +{
>>> +     char *p;
>>> +
>>> +        if (unlikely(!s))
>>> +                return NULL;
>>
>> Using spaces for indentation? See scripts/checkpatch.pl.
>
> Not on purpose. Thanks for spotting it.
>
>>> +
>>> +     for (p = s; *p; p++)
>>> +             *p = tolower(*p);
>>> +
>>> +     return s;
>>
>> Why does it return a value? Could be void?
>
> It could be void, but I thought that would make the function's use
> less flexible. As is, the return value is there if anybody wants it,
> but it can be ignored if it is not needed. Also, it seems customary
> for string functions to be returning the string that was passed in.
>
> I'll change it to void if there are strong opinions leaning that way.
> Personally, I like that it returns a char * better.

I don't have strong opinions on this. Just a general aversion to
returning something redundant. Avoids questions like, does it allocate a
new string, should I use the return value instead of the string I passed
in, should I check the return value or can I ignore it, should I check
both the string I pass in and the return value for != NULL, etc. But I
could be persuaded either way.

BR,
Jani.


>
>> BR,
>> Jani.
>>
>>> +}
>>> +EXPORT_SYMBOL(strtolower);
>>
>> --
>> Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1435626 — Re: [PATCH 1/6] lib: string: add function strtolower()

On Fri, Jul 01 2016, Markus Mayer <mmayer@broadcom.com> wrote:

> Add a function called strtolower() to convert strings to lower case
> in-place, overwriting the original string.
>
> This seems to be a recurring requirement in the kernel that is
> currently being solved by several duplicated implementations doing the
> same thing.
>
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> ---
>  include/linux/string.h |  1 +
>  lib/string.c           | 14 ++++++++++++++
>  2 files changed, 15 insertions(+)
>
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 26b6f6a..aad605e 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -116,6 +116,7 @@ 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 *strtolower(char *s);
>  
>  extern void kfree_const(const void *x);
>  
> diff --git a/lib/string.c b/lib/string.c
> index ed83562..6e3b560 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -952,3 +952,17 @@ char *strreplace(char *s, char old, char new)
>  	return s;
>  }
>  EXPORT_SYMBOL(strreplace);
> +
> +char *strtolower(char *s)
> +{
> +	char *p;
> +
> +        if (unlikely(!s))
> +                return NULL;
> +
> +	for (p = s; *p; p++)
> +		*p = tolower(*p);
> +
> +	return s;
> +}
> +EXPORT_SYMBOL(strtolower);

A few suggestions: 

- Make the function take separate src and dst parameters, making it explicitly
  allowed to pass the same value (but not other kinds of overlap, of
  course). That way one can avoid "strcpy(dst, src); strtolower(dst);".

- Drop the NULL check. If someone does "foo->bar = something;
  strtolower(foo->bar); put foo in a global data structure...", the
  dereference of foo->bar may happen much later. Doing the NULL deref
  sooner means it's much easier to find and fix the bug. (Also, other
  str* and mem* functions don't usually check for NULL).

- While it's true that strcpy and memcpy by definition return dst, that's
  mostly useless. If you want it to return anything, please make it
  something that might be used - for example, having stpcpy semantics
  (returning a pointer to dst's terminating \0) means a caller might avoid
  a strlen call.

- Maybe do strtoupper while you're at it. Quick grepping didn't find any
  use for the copy-while-lowercasing, but copy-while-uppercasing can at
  least be used in drivers/acpi/acpica/nsrepair2.c,
  drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c,
  drivers/power/power_supply_sysfs.c along with a bunch of inplace
  uppercasing.


Rasmus

[toc] | [prev] | [next] | [standalone]

#1436584 — Re: [PATCH 1/6] lib: string: add function strtolower()

On 1 July 2016 at 14:08, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:

> A few suggestions:
>
> - Make the function take separate src and dst parameters, making it explicitly
>   allowed to pass the same value (but not other kinds of overlap, of
>   course). That way one can avoid "strcpy(dst, src); strtolower(dst);".
>
> - Drop the NULL check. If someone does "foo->bar = something;
>   strtolower(foo->bar); put foo in a global data structure...", the
>   dereference of foo->bar may happen much later. Doing the NULL deref
>   sooner means it's much easier to find and fix the bug. (Also, other
>   str* and mem* functions don't usually check for NULL).
>
> - While it's true that strcpy and memcpy by definition return dst, that's
>   mostly useless. If you want it to return anything, please make it
>   something that might be used - for example, having stpcpy semantics
>   (returning a pointer to dst's terminating \0) means a caller might avoid
>   a strlen call.
>
> - Maybe do strtoupper while you're at it. Quick grepping didn't find any
>   use for the copy-while-lowercasing, but copy-while-uppercasing can at
>   least be used in drivers/acpi/acpica/nsrepair2.c,
>   drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c,
>   drivers/power/power_supply_sysfs.c along with a bunch of inplace
>   uppercasing.
>
>
> Rasmus

Thanks for the suggestions to you and Jani. Based on the feedback I
received, I am reworking the series now and will post v2 probably
tomorrow.

Regards,
-Markus

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web