2018-08-17 20:22:10

by Harshit Jain

[permalink] [raw]
Subject: [PATCH] scripts: unifdef: fix stringop-truncation warning

While compiling the mainline kernel in android env with latest gcc 8 the compiler threw this warning Thanks to the advantage of improved static analysis in newer gcc we are now able to
see this warning this patch resolves the detected compiler warnings.

Signed-off-by: Harshit Jain <[email protected]>
---
scripts/unifdef.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/unifdef.c b/scripts/unifdef.c
index 7493c0ee51cc..985649805cdd 100644
--- a/scripts/unifdef.c
+++ b/scripts/unifdef.c
@@ -450,7 +450,7 @@ static void Idrop (void) { Fdrop(); ignoreon(); }
static void Itrue (void) { Ftrue(); ignoreon(); }
static void Ifalse(void) { Ffalse(); ignoreon(); }
/* modify this line */
-static void Mpass (void) { strncpy(keyword, "if ", 4); Pelif(); }
+static void Mpass (void) { memcpy(keyword, "if ", 4); Pelif(); }
static void Mtrue (void) { keywordedit("else"); state(IS_TRUE_MIDDLE); }
static void Melif (void) { keywordedit("endif"); state(IS_FALSE_TRAILER); }
static void Melse (void) { keywordedit("endif"); state(IS_FALSE_ELSE); }
--
2.18.0



2018-08-21 15:40:01

by Tony Finch

[permalink] [raw]
Subject: Re: [PATCH] scripts: unifdef: fix stringop-truncation warning

Harshit Jain <[email protected]> wrote:

> /* modify this line */
> -static void Mpass (void) { strncpy(keyword, "if ", 4); Pelif(); }
> +static void Mpass (void) { memcpy(keyword, "if ", 4); Pelif(); }

Yes, this is a good improvement, but you also need to update the comment
to match the code.

Tony.
--
f.anthony.n.finch <[email protected]> http://dotat.at/
Wight, Portland, Plymouth: Westerly 3 or 4, occasionally 5 in north. Slight,
occasionally moderate. Drizzle, fog banks. Moderate, occasionally very poor.

2018-08-23 16:15:37

by Tony Finch

[permalink] [raw]
Subject: Re: [PATCH] scripts: unifdef: fix stringop-truncation warning

Harshit Jain <[email protected]> wrote:

> Hey! tony thanks for reviewing, pardon me i wasn't able to get your context
> can you please elaborate a little what you are trying to say?

There's a big block comment above the state machine code which tries to
explain what the blazes is going on. The comment includes an explanation
of the weird strncpy() call, which you need to update to talk about
memcpy() instead.

(The code is heavily compactified because I wrote it on a laptop with a
small screen and I needed to be able to see it all to understand it!)

Tony.
--
f.anthony.n.finch <[email protected]> http://dotat.at/
South Fitzroy: Northerly or northeasterly 5 or 6. Slight or moderate.
Occasional drizzle. Good, occasionally poor at first.

2018-09-14 17:13:56

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH] scripts: unifdef: fix stringop-truncation warning

Hi Harshit,

On Fri, Sep 14, 2018 at 12:44 PM, Harshit Jain
<[email protected]> wrote:
> From: Harshit Jain <[email protected]>
>
> * This commit resolves the following warning when the mainline kernel is build with the android environment.

Typo: built

>
> -> warning :-> https://gist.github.com/dev-harsh1998/757427b16a58f5498db3d87212a9651b

Try to avoid links to external pages (specially if they are only 2 lines :-).

>
> * This warning is persistant in all the currently maintained android kernel i.e 3.18, 4.4, 4.9, 4.14.

Typos: persistent, kernels, i.e.

>
> Signed-off-by: Harshit Jain <[email protected]>
> ---
> scripts/unifdef.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/unifdef.c b/scripts/unifdef.c
> index 7493c0ee51cc..4ce008eda362 100644
> --- a/scripts/unifdef.c
> +++ b/scripts/unifdef.c
> @@ -395,8 +395,8 @@ usage(void)
> * When we have processed a group that starts off with a known-false
> * #if/#elif sequence (which has therefore been deleted) followed by a
> * #elif that we don't understand and therefore must keep, we edit the
> - * latter into a #if to keep the nesting correct. We use strncpy() to
> - * overwrite the 4 byte token "elif" with "if " without a '\0' byte.
> + * latter into a #if to keep the nesting correct. We use the memcpy()
> + * from the string header overwrite the 4 byte token "elif" with "if ".
> *
> * When we find a true #elif in a group, the following block will
> * always be kept and the rest of the sequence after the next #elif or
> @@ -450,7 +450,7 @@ static void Idrop (void) { Fdrop(); ignoreon(); }
> static void Itrue (void) { Ftrue(); ignoreon(); }
> static void Ifalse(void) { Ffalse(); ignoreon(); }
> /* modify this line */
> -static void Mpass (void) { strncpy(keyword, "if ", 4); Pelif(); }
> +static void Mpass (void) { memcpy(keyword, "if ", 4); Pelif(); }

Seems like keyword shouldn't use __attribute__((nonstring)), so this
looks right.

Cheers,
Miguel

> static void Mtrue (void) { keywordedit("else"); state(IS_TRUE_MIDDLE); }
> static void Melif (void) { keywordedit("endif"); state(IS_FALSE_TRAILER); }
> static void Melse (void) { keywordedit("endif"); state(IS_FALSE_ELSE); }
> --
> 2.18.0
>

2018-09-14 17:19:14

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH] scripts: unifdef: fix stringop-truncation warning

On Fri, Sep 14, 2018 at 7:12 PM, Miguel Ojeda
<[email protected]> wrote:
> Hi Harshit,

Another two nitpicks:

* Since this is the second version of the patch, write "[PATCH v2]"
in the title instead; that way people know you have sent an earlier
version. On top of that, you should explain the differences between
each version after the "---" line (i.e. the part that does not belong
to the commit message).

* You may want to write down who you CC'd (e.g. Tony and the stable
mailing list) in "Cc: ..." lines before the Signed-off-by line.

Thanks for the patch!

Hope that helps,
Miguel

>
> On Fri, Sep 14, 2018 at 12:44 PM, Harshit Jain
> <[email protected]> wrote:
>> From: Harshit Jain <[email protected]>
>>
>> * This commit resolves the following warning when the mainline kernel is build with the android environment.
>
> Typo: built
>
>>
>> -> warning :-> https://gist.github.com/dev-harsh1998/757427b16a58f5498db3d87212a9651b
>
> Try to avoid links to external pages (specially if they are only 2 lines :-).
>
>>
>> * This warning is persistant in all the currently maintained android kernel i.e 3.18, 4.4, 4.9, 4.14.
>
> Typos: persistent, kernels, i.e.
>
>>
>> Signed-off-by: Harshit Jain <[email protected]>
>> ---
>> scripts/unifdef.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/scripts/unifdef.c b/scripts/unifdef.c
>> index 7493c0ee51cc..4ce008eda362 100644
>> --- a/scripts/unifdef.c
>> +++ b/scripts/unifdef.c
>> @@ -395,8 +395,8 @@ usage(void)
>> * When we have processed a group that starts off with a known-false
>> * #if/#elif sequence (which has therefore been deleted) followed by a
>> * #elif that we don't understand and therefore must keep, we edit the
>> - * latter into a #if to keep the nesting correct. We use strncpy() to
>> - * overwrite the 4 byte token "elif" with "if " without a '\0' byte.
>> + * latter into a #if to keep the nesting correct. We use the memcpy()
>> + * from the string header overwrite the 4 byte token "elif" with "if ".
>> *
>> * When we find a true #elif in a group, the following block will
>> * always be kept and the rest of the sequence after the next #elif or
>> @@ -450,7 +450,7 @@ static void Idrop (void) { Fdrop(); ignoreon(); }
>> static void Itrue (void) { Ftrue(); ignoreon(); }
>> static void Ifalse(void) { Ffalse(); ignoreon(); }
>> /* modify this line */
>> -static void Mpass (void) { strncpy(keyword, "if ", 4); Pelif(); }
>> +static void Mpass (void) { memcpy(keyword, "if ", 4); Pelif(); }
>
> Seems like keyword shouldn't use __attribute__((nonstring)), so this
> looks right.
>
> Cheers,
> Miguel
>
>> static void Mtrue (void) { keywordedit("else"); state(IS_TRUE_MIDDLE); }
>> static void Melif (void) { keywordedit("endif"); state(IS_FALSE_TRAILER); }
>> static void Melse (void) { keywordedit("endif"); state(IS_FALSE_ELSE); }
>> --
>> 2.18.0
>>