[PATCH v3] x86/platform/uv: refactor deprecated strcpy and strncpy

[PATCH v3] x86/platform/uv: refactor deprecated strcpy and strncpy

[Justin Stitt]

Both `strncpy` and `strcpy` are deprecated for use on NUL-terminated
destination strings [1].

We can see that `arg` and `uv_nmi_action` are expected to be
NUL-terminated strings due to their use within `strcmp()` and format
strings respectively.

With this in mind, a suitable replacement is `strscpy` [2] due to the
fact that it guarantees NUL-termination on its destination buffer
argument which is _not_ the case for `strncpy` or `strcpy`!

In this case, we can drop both the forced NUL-termination and the `... -1` from:
|       strncpy(arg, val, ACTION_LEN - 1);
as `strscpy` implicitly has this behavior.

Link: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Changes in v3:
- Use sizeof instead of strlen (thanks Andy and Dimitri)
- Drop unrelated changes regarding strnchrnul (thanks Hans)
- Link to v2: https://lore.kernel.org/r/20230824-strncpy-arch-x86-platform-uv-uv_nmi-v2-1-e16d9a3ec570@google.com

Changes in v2:
- use `sizeof` on destination string instead of `strlen` (thanks Andy, Kees and Dimitri)
- refactor code to remove potential new-line chars (thanks Yang Yang and Andy)
- Link to v1: https://lore.kernel.org/r/20230822-strncpy-arch-x86-platform-uv-uv_nmi-v1-1-931f2943de0d@google.com
---
Note: build-tested only
---
 arch/x86/platform/uv/uv_nmi.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/platform/uv/uv_nmi.c b/arch/x86/platform/uv/uv_nmi.c
index a60af0230e27..dd30fb2baf6c 100644
--- a/arch/x86/platform/uv/uv_nmi.c
+++ b/arch/x86/platform/uv/uv_nmi.c
@@ -205,8 +205,7 @@ static int param_set_action(const char *val, const struct kernel_param *kp)
 	char arg[ACTION_LEN], *p;
 
 	/* (remove possible '\n') */
-	strncpy(arg, val, ACTION_LEN - 1);
-	arg[ACTION_LEN - 1] = '\0';
+	strscpy(arg, val, sizeof(arg));
 	p = strchr(arg, '\n');
 	if (p)
 		*p = '\0';
@@ -216,7 +215,7 @@ static int param_set_action(const char *val, const struct kernel_param *kp)
 			break;
 
 	if (i < n) {
-		strcpy(uv_nmi_action, arg);
+		strscpy(uv_nmi_action, arg, sizeof(uv_nmi_action));
 		pr_info("UV: New NMI action:%s\n", uv_nmi_action);
 		return 0;
 	}
@@ -959,7 +958,7 @@ static int uv_handle_nmi(unsigned int reason, struct pt_regs *regs)
 
 		/* Unexpected return, revert action to "dump" */
 		if (master)
-			strncpy(uv_nmi_action, "dump", strlen(uv_nmi_action));
+			strscpy(uv_nmi_action, "dump", sizeof(uv_nmi_action));
 	}
 
 	/* Pause as all CPU's enter the NMI handler */

---
base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
change-id: 20230822-strncpy-arch-x86-platform-uv-uv_nmi-474e5295c2c1

Best regards,
--
Justin Stitt <justinstitt@google.com>

Re: [PATCH v3] x86/platform/uv: refactor deprecated strcpy and strncpy

[Hans de Goede]

Hi,

On 9/5/23 23:54, Justin Stitt wrote:
> Both `strncpy` and `strcpy` are deprecated for use on NUL-terminated
> destination strings [1].
> 
> We can see that `arg` and `uv_nmi_action` are expected to be
> NUL-terminated strings due to their use within `strcmp()` and format
> strings respectively.
> 
> With this in mind, a suitable replacement is `strscpy` [2] due to the
> fact that it guarantees NUL-termination on its destination buffer
> argument which is _not_ the case for `strncpy` or `strcpy`!
> 
> In this case, we can drop both the forced NUL-termination and the `... -1` from:
> |       strncpy(arg, val, ACTION_LEN - 1);
> as `strscpy` implicitly has this behavior.
> 
> Link: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1]
> Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
> Link: https://github.com/KSPP/linux/issues/90
> Cc: linux-hardening@vger.kernel.org
> Signed-off-by: Justin Stitt <justinstitt@google.com>
> ---
> Changes in v3:
> - Use sizeof instead of strlen (thanks Andy and Dimitri)
> - Drop unrelated changes regarding strnchrnul (thanks Hans)
> - Link to v2: https://lore.kernel.org/r/20230824-strncpy-arch-x86-platform-uv-uv_nmi-v2-1-e16d9a3ec570@google.com

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> 
> Changes in v2:
> - use `sizeof` on destination string instead of `strlen` (thanks Andy, Kees and Dimitri)
> - refactor code to remove potential new-line chars (thanks Yang Yang and Andy)
> - Link to v1: https://lore.kernel.org/r/20230822-strncpy-arch-x86-platform-uv-uv_nmi-v1-1-931f2943de0d@google.com
> ---
> Note: build-tested only
> ---
>  arch/x86/platform/uv/uv_nmi.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/platform/uv/uv_nmi.c b/arch/x86/platform/uv/uv_nmi.c
> index a60af0230e27..dd30fb2baf6c 100644
> --- a/arch/x86/platform/uv/uv_nmi.c
> +++ b/arch/x86/platform/uv/uv_nmi.c
> @@ -205,8 +205,7 @@ static int param_set_action(const char *val, const struct kernel_param *kp)
>  	char arg[ACTION_LEN], *p;
>  
>  	/* (remove possible '\n') */
> -	strncpy(arg, val, ACTION_LEN - 1);
> -	arg[ACTION_LEN - 1] = '\0';
> +	strscpy(arg, val, sizeof(arg));
>  	p = strchr(arg, '\n');
>  	if (p)
>  		*p = '\0';
> @@ -216,7 +215,7 @@ static int param_set_action(const char *val, const struct kernel_param *kp)
>  			break;
>  
>  	if (i < n) {
> -		strcpy(uv_nmi_action, arg);
> +		strscpy(uv_nmi_action, arg, sizeof(uv_nmi_action));
>  		pr_info("UV: New NMI action:%s\n", uv_nmi_action);
>  		return 0;
>  	}
> @@ -959,7 +958,7 @@ static int uv_handle_nmi(unsigned int reason, struct pt_regs *regs)
>  
>  		/* Unexpected return, revert action to "dump" */
>  		if (master)
> -			strncpy(uv_nmi_action, "dump", strlen(uv_nmi_action));
> +			strscpy(uv_nmi_action, "dump", sizeof(uv_nmi_action));
>  	}
>  
>  	/* Pause as all CPU's enter the NMI handler */
> 
> ---
> base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
> change-id: 20230822-strncpy-arch-x86-platform-uv-uv_nmi-474e5295c2c1
> 
> Best regards,
> --
> Justin Stitt <justinstitt@google.com>
> 

Re: [PATCH v3] x86/platform/uv: refactor deprecated strcpy and strncpy

[Ingo Molnar]

* Justin Stitt <justinstitt@google.com> wrote:

> Both `strncpy` and `strcpy` are deprecated for use on NUL-terminated
> destination strings [1].
> 
> We can see that `arg` and `uv_nmi_action` are expected to be
> NUL-terminated strings due to their use within `strcmp()` and format
> strings respectively.
> 
> With this in mind, a suitable replacement is `strscpy` [2] due to the
> fact that it guarantees NUL-termination on its destination buffer
> argument which is _not_ the case for `strncpy` or `strcpy`!
> 
> In this case, we can drop both the forced NUL-termination and the `... -1` from:
> |       strncpy(arg, val, ACTION_LEN - 1);
> as `strscpy` implicitly has this behavior.
> 
> Link: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1]
> Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
> Link: https://github.com/KSPP/linux/issues/90
> Cc: linux-hardening@vger.kernel.org
> Signed-off-by: Justin Stitt <justinstitt@google.com>

>  arch/x86/platform/uv/uv_nmi.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)

Note that this commit is already upstream:

  1e6f01f72855 ("x86/platform/uv: Refactor code using deprecated strcpy()/strncpy() interfaces to use strscpy()")

Below is the delta your v3 patch has compared to what is upstream - is it 
really necessary to open code it, instead of using strnchrnul() as your 
original patch did? Am I missing anything here?

Thanks,

	Ingo

--- a/arch/x86/platform/uv/uv_nmi.c
+++ b/arch/x86/platform/uv/uv_nmi.c
@@ -202,10 +202,13 @@ static int param_set_action(const char *val, const struct kernel_param *kp)
 {
 	int i;
 	int n = ARRAY_SIZE(valid_acts);
-	char arg[ACTION_LEN];
+	char arg[ACTION_LEN], *p;
 
 	/* (remove possible '\n') */
-	strscpy(arg, val, strnchrnul(val, sizeof(arg)-1, '\n') - val + 1);
+	strscpy(arg, val, sizeof(arg));
+	p = strchr(arg, '\n');
+	if (p)
+		*p = '\0';
 
 	for (i = 0; i < n; i++)
 		if (!strcmp(arg, valid_acts[i].action))

Re: [PATCH v3] x86/platform/uv: refactor deprecated strcpy and strncpy

[Hans de Goede]

Hi Ingo,

On 9/6/23 14:10, Ingo Molnar wrote:
> 
> * Justin Stitt <justinstitt@google.com> wrote:
> 
>> Both `strncpy` and `strcpy` are deprecated for use on NUL-terminated
>> destination strings [1].
>>
>> We can see that `arg` and `uv_nmi_action` are expected to be
>> NUL-terminated strings due to their use within `strcmp()` and format
>> strings respectively.
>>
>> With this in mind, a suitable replacement is `strscpy` [2] due to the
>> fact that it guarantees NUL-termination on its destination buffer
>> argument which is _not_ the case for `strncpy` or `strcpy`!
>>
>> In this case, we can drop both the forced NUL-termination and the `... -1` from:
>> |       strncpy(arg, val, ACTION_LEN - 1);
>> as `strscpy` implicitly has this behavior.
>>
>> Link: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1]
>> Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
>> Link: https://github.com/KSPP/linux/issues/90
>> Cc: linux-hardening@vger.kernel.org
>> Signed-off-by: Justin Stitt <justinstitt@google.com>
> 
>>  arch/x86/platform/uv/uv_nmi.c | 7 +++----
>>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> Note that this commit is already upstream:
> 
>   1e6f01f72855 ("x86/platform/uv: Refactor code using deprecated strcpy()/strncpy() interfaces to use strscpy()")
> 
> Below is the delta your v3 patch has compared to what is upstream - is it 
> really necessary to open code it, instead of using strnchrnul() as your 
> original patch did? Am I missing anything here?

The new version is a result of a review from my because IMHO:

	strscpy(arg, val, strnchrnul(val, sizeof(arg)-1, '\n') - val + 1);

Is really unreadable / really hard to reason about if
this is actually correct and does not contain any
of by 1 bugs.

Note that the diff of v3 compared to the code before v2 landed is
actually smaller now and actually matches the subject of:
"refactor deprecated strcpy and strncpy"

Where as v2 actually touches more code / refactor things
which fall outside of a "one change per patch" approach.
The:

	p = strchr(arg, '\n');
	if (p)
		*p = '\0';

was already there before v2 landed.

I also suggested to do a follow up patch to change things to:

	strscpy(arg, val, sizeof(arg));
	p = strchrnul(arg, '\n');
	*p = '\0';

Which IMHO is much more readable then what has landed
now. But since v2 has already landed I guess the best
thing is just to stick with what we have upstream now...

Regards,

Hans





> 
> Thanks,
> 
> 	Ingo
> 
> --- a/arch/x86/platform/uv/uv_nmi.c
> +++ b/arch/x86/platform/uv/uv_nmi.c
> @@ -202,10 +202,13 @@ static int param_set_action(const char *val, const struct kernel_param *kp)
>  {
>  	int i;
>  	int n = ARRAY_SIZE(valid_acts);
> -	char arg[ACTION_LEN];
> +	char arg[ACTION_LEN], *p;
>  
>  	/* (remove possible '\n') */
> -	strscpy(arg, val, strnchrnul(val, sizeof(arg)-1, '\n') - val + 1);
> +	strscpy(arg, val, sizeof(arg));
> +	p = strchr(arg, '\n');
> +	if (p)
> +		*p = '\0';
>  
>  	for (i = 0; i < n; i++)
>  		if (!strcmp(arg, valid_acts[i].action))
> 

Re: [PATCH v3] x86/platform/uv: refactor deprecated strcpy and strncpy

[Andy Shevchenko]

On Wed, Sep 6, 2023 at 3:16 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 9/6/23 14:10, Ingo Molnar wrote:

>         strscpy(arg, val, strnchrnul(val, sizeof(arg)-1, '\n') - val + 1);

If you want to make it short and more readable, you can use

  strscpy(arg, val, sizeof(arg));
  strreplace(arg, '\n', '\0');

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3] x86/platform/uv: refactor deprecated strcpy and strncpy

[Ingo Molnar]

* Hans de Goede <hdegoede@redhat.com> wrote:

> Hi Ingo,
> 
> On 9/6/23 14:10, Ingo Molnar wrote:
> > 
> > * Justin Stitt <justinstitt@google.com> wrote:
> > 
> >> Both `strncpy` and `strcpy` are deprecated for use on NUL-terminated
> >> destination strings [1].
> >>
> >> We can see that `arg` and `uv_nmi_action` are expected to be
> >> NUL-terminated strings due to their use within `strcmp()` and format
> >> strings respectively.
> >>
> >> With this in mind, a suitable replacement is `strscpy` [2] due to the
> >> fact that it guarantees NUL-termination on its destination buffer
> >> argument which is _not_ the case for `strncpy` or `strcpy`!
> >>
> >> In this case, we can drop both the forced NUL-termination and the `... -1` from:
> >> |       strncpy(arg, val, ACTION_LEN - 1);
> >> as `strscpy` implicitly has this behavior.
> >>
> >> Link: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1]
> >> Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
> >> Link: https://github.com/KSPP/linux/issues/90
> >> Cc: linux-hardening@vger.kernel.org
> >> Signed-off-by: Justin Stitt <justinstitt@google.com>
> > 
> >>  arch/x86/platform/uv/uv_nmi.c | 7 +++----
> >>  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > Note that this commit is already upstream:
> > 
> >   1e6f01f72855 ("x86/platform/uv: Refactor code using deprecated strcpy()/strncpy() interfaces to use strscpy()")
> > 
> > Below is the delta your v3 patch has compared to what is upstream - is it 
> > really necessary to open code it, instead of using strnchrnul() as your 
> > original patch did? Am I missing anything here?
> 
> The new version is a result of a review from my because IMHO:
> 
> 	strscpy(arg, val, strnchrnul(val, sizeof(arg)-1, '\n') - val + 1);
> 
> Is really unreadable / really hard to reason about if
> this is actually correct and does not contain any
> of by 1 bugs.
> 
> Note that the diff of v3 compared to the code before v2 landed is
> actually smaller now and actually matches the subject of:
> "refactor deprecated strcpy and strncpy"
> 
> Where as v2 actually touches more code / refactor things
> which fall outside of a "one change per patch" approach.
> The:
> 
> 	p = strchr(arg, '\n');
> 	if (p)
> 		*p = '\0';
> 
> was already there before v2 landed.
> 
> I also suggested to do a follow up patch to change things to:
> 
> 	strscpy(arg, val, sizeof(arg));
> 	p = strchrnul(arg, '\n');
> 	*p = '\0';
> 
> Which IMHO is much more readable then what has landed
> now. But since v2 has already landed I guess the best
> thing is just to stick with what we have upstream now...

Well, how about we do a delta patch with all the changes
you suggested? I'm all for readability.

Thanks,

	Ingo

Re: [PATCH v3] x86/platform/uv: refactor deprecated strcpy and strncpy

[Steve Wahl]

On Wed, Sep 06, 2023 at 04:09:01PM +0200, Ingo Molnar wrote:
> 
> * Hans de Goede <hdegoede@redhat.com> wrote:
> 
> > Hi Ingo,
> > 
> > On 9/6/23 14:10, Ingo Molnar wrote:
> > > 
> > > * Justin Stitt <justinstitt@google.com> wrote:
> > > 
> > >> Both `strncpy` and `strcpy` are deprecated for use on NUL-terminated
> > >> destination strings [1].
> > >>
> > >> We can see that `arg` and `uv_nmi_action` are expected to be
> > >> NUL-terminated strings due to their use within `strcmp()` and format
> > >> strings respectively.
> > >>
> > >> With this in mind, a suitable replacement is `strscpy` [2] due to the
> > >> fact that it guarantees NUL-termination on its destination buffer
> > >> argument which is _not_ the case for `strncpy` or `strcpy`!
> > >>
> > >> In this case, we can drop both the forced NUL-termination and the `... -1` from:
> > >> |       strncpy(arg, val, ACTION_LEN - 1);
> > >> as `strscpy` implicitly has this behavior.
> > >>
> > >> Link: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1]
> > >> Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
> > >> Link: https://github.com/KSPP/linux/issues/90
> > >> Cc: linux-hardening@vger.kernel.org
> > >> Signed-off-by: Justin Stitt <justinstitt@google.com>
> > > 
> > >>  arch/x86/platform/uv/uv_nmi.c | 7 +++----
> > >>  1 file changed, 3 insertions(+), 4 deletions(-)
> > > 
> > > Note that this commit is already upstream:
> > > 
> > >   1e6f01f72855 ("x86/platform/uv: Refactor code using deprecated strcpy()/strncpy() interfaces to use strscpy()")
> > > 
> > > Below is the delta your v3 patch has compared to what is upstream - is it 
> > > really necessary to open code it, instead of using strnchrnul() as your 
> > > original patch did? Am I missing anything here?
> > 
> > The new version is a result of a review from my because IMHO:
> > 
> > 	strscpy(arg, val, strnchrnul(val, sizeof(arg)-1, '\n') - val + 1);
> > 
> > Is really unreadable / really hard to reason about if
> > this is actually correct and does not contain any
> > of by 1 bugs.
> > 
> > Note that the diff of v3 compared to the code before v2 landed is
> > actually smaller now and actually matches the subject of:
> > "refactor deprecated strcpy and strncpy"
> > 
> > Where as v2 actually touches more code / refactor things
> > which fall outside of a "one change per patch" approach.
> > The:
> > 
> > 	p = strchr(arg, '\n');
> > 	if (p)
> > 		*p = '\0';
> > 
> > was already there before v2 landed.
> > 
> > I also suggested to do a follow up patch to change things to:
> > 
> > 	strscpy(arg, val, sizeof(arg));
> > 	p = strchrnul(arg, '\n');
> > 	*p = '\0';
> > 
> > Which IMHO is much more readable then what has landed
> > now. But since v2 has already landed I guess the best
> > thing is just to stick with what we have upstream now...
> 
> Well, how about we do a delta patch with all the changes
> you suggested? I'm all for readability.

For whatever it's worth, I vote in favor of adopting an increased
readability version.

I was on vacation when the patch came through, and by the time I
reviewed it it was already accepted.  I still puzzled through the
-1/+1 stuff to be sure it functioned correctly; since it worked and
was already accepted, I let it go.

When Hans' comments on readability later came through, I was thinking
"Yes, he's exactly right! Why, when I worked so hard on verifying that
the code worked properly, did it not occur to me to suggest re-writing
this in a simpler fashion to make the intent clear?"

Thanks,

--> Steve Wahl

-- 
Steve Wahl, Hewlett Packard Enterprise

Re: [PATCH v3] x86/platform/uv: refactor deprecated strcpy and strncpy

[Hans de Goede]

Hi,

On 9/6/23 16:09, Ingo Molnar wrote:
> 
> * Hans de Goede <hdegoede@redhat.com> wrote:
> 
>> Hi Ingo,
>>
>> On 9/6/23 14:10, Ingo Molnar wrote:
>>>
>>> * Justin Stitt <justinstitt@google.com> wrote:
>>>
>>>> Both `strncpy` and `strcpy` are deprecated for use on NUL-terminated
>>>> destination strings [1].
>>>>
>>>> We can see that `arg` and `uv_nmi_action` are expected to be
>>>> NUL-terminated strings due to their use within `strcmp()` and format
>>>> strings respectively.
>>>>
>>>> With this in mind, a suitable replacement is `strscpy` [2] due to the
>>>> fact that it guarantees NUL-termination on its destination buffer
>>>> argument which is _not_ the case for `strncpy` or `strcpy`!
>>>>
>>>> In this case, we can drop both the forced NUL-termination and the `... -1` from:
>>>> |       strncpy(arg, val, ACTION_LEN - 1);
>>>> as `strscpy` implicitly has this behavior.
>>>>
>>>> Link: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1]
>>>> Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
>>>> Link: https://github.com/KSPP/linux/issues/90
>>>> Cc: linux-hardening@vger.kernel.org
>>>> Signed-off-by: Justin Stitt <justinstitt@google.com>
>>>
>>>>  arch/x86/platform/uv/uv_nmi.c | 7 +++----
>>>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>>
>>> Note that this commit is already upstream:
>>>
>>>   1e6f01f72855 ("x86/platform/uv: Refactor code using deprecated strcpy()/strncpy() interfaces to use strscpy()")
>>>
>>> Below is the delta your v3 patch has compared to what is upstream - is it 
>>> really necessary to open code it, instead of using strnchrnul() as your 
>>> original patch did? Am I missing anything here?
>>
>> The new version is a result of a review from my because IMHO:
>>
>> 	strscpy(arg, val, strnchrnul(val, sizeof(arg)-1, '\n') - val + 1);
>>
>> Is really unreadable / really hard to reason about if
>> this is actually correct and does not contain any
>> of by 1 bugs.
>>
>> Note that the diff of v3 compared to the code before v2 landed is
>> actually smaller now and actually matches the subject of:
>> "refactor deprecated strcpy and strncpy"
>>
>> Where as v2 actually touches more code / refactor things
>> which fall outside of a "one change per patch" approach.
>> The:
>>
>> 	p = strchr(arg, '\n');
>> 	if (p)
>> 		*p = '\0';
>>
>> was already there before v2 landed.
>>
>> I also suggested to do a follow up patch to change things to:
>>
>> 	strscpy(arg, val, sizeof(arg));
>> 	p = strchrnul(arg, '\n');
>> 	*p = '\0';
>>
>> Which IMHO is much more readable then what has landed
>> now. But since v2 has already landed I guess the best
>> thing is just to stick with what we have upstream now...
> 
> Well, how about we do a delta patch with all the changes
> you suggested? I'm all for readability.

So I started doing this and notices that all the string
manipulation + parsing done here is really just a DYI
implementation of sysfs_match_string().

So I have prepared a patch to switch to sysfs_match_string(),
which completely removes the need to make a copy of the val
string.

I'll submit the patch right after this email.

Regards,

Hans

Re: [PATCH v3] x86/platform/uv: refactor deprecated strcpy and strncpy

[Ingo Molnar]

* Hans de Goede <hdegoede@redhat.com> wrote:

> >> Which IMHO is much more readable then what has landed now. But since 
> >> v2 has already landed I guess the best thing is just to stick with 
> >> what we have upstream now...
> > 
> > Well, how about we do a delta patch with all the changes you suggested? 
> > I'm all for readability.
> 
> So I started doing this and notices that all the string manipulation + 
> parsing done here is really just a DYI implementation of 
> sysfs_match_string().
> 
> So I have prepared a patch to switch to sysfs_match_string(), which 
> completely removes the need to make a copy of the val string.
> 
> I'll submit the patch right after this email.

Thank you - that looks a far more thorough cleanup indeed.

	Ingo