[PATCH next] Fix the build failed caused by -Wstringop-overflow

[PATCH next] Fix the build failed caused by -Wstringop-overflow

[Wenyu Huang]

gcc version: gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0

I use this version of gcc to compile, and the compilation fails. It said
that "error: writing 1 byte into a region of size 0
[-Werror=stringop-overflow=]".

This patch is to fix it to build successfully.

Fixes: 89741e7e42f6 ("Makefile: Enable -Wstringop-overflow globally")
Signed-off-by: Wenyu Huang <huangwenyu5@huawei.com>
---
 lib/vsprintf.c | 4 ++++
 mm/mempolicy.c | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 3e3733a7084f..7a247cfecd61 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -2887,12 +2887,14 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
 	}
 
 out:
+#pragma GCC diagnostic ignored "-Wstringop-overflow"
 	if (size > 0) {
 		if (str < end)
 			*str = '\0';
 		else
 			end[-1] = '\0';
 	}
+#pragma GCC diagnostic pop
 
 	/* the trailing null byte doesn't count towards the total */
 	return str-buf;
@@ -3385,12 +3387,14 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
 	} /* while(*fmt) */
 
 out:
+#pragma GCC diagnostic ignored "-Wstringop-overflow"
 	if (size > 0) {
 		if (str < end)
 			*str = '\0';
 		else
 			end[-1] = '\0';
 	}
+#pragma GCC diagnostic pop
 
 #undef get_arg
 
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 10a590ee1c89..6c8433228c71 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -3000,6 +3000,7 @@ int mpol_parse_str(char *str, struct mempolicy **mpol)
 	err = 0;
 
 out:
+#pragma GCC diagnostic ignored "-Wstringop-overflow"
 	/* Restore string for error message */
 	if (nodelist)
 		*--nodelist = ':';
@@ -3008,6 +3009,7 @@ int mpol_parse_str(char *str, struct mempolicy **mpol)
 	if (!err)
 		*mpol = new;
 	return err;
+#pragma GCC diagnostic pop
 }
 #endif /* CONFIG_TMPFS */
 
-- 
2.34.1

Re: [PATCH next] Fix the build failed caused by -Wstringop-overflow

[Gustavo A. R. Silva]

On 11/30/23 04:57, Wenyu Huang wrote:
> gcc version: gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
> 
> I use this version of gcc to compile, and the compilation fails. It said
> that "error: writing 1 byte into a region of size 0
> [-Werror=stringop-overflow=]".
> 
> This patch is to fix it to build successfully.
> 

This doesn't really fix anything. GCC 11 is buggy and the issues you
see are false positives that doesn't show up in other versions of
the compiler.

> Fixes: 89741e7e42f6 ("Makefile: Enable -Wstringop-overflow globally")

The commit ID is from a patch that's currently in linux-next, which
does not guarantee it's a stable commit. So, it shouldn't be used
for any tag in any changelog text. In fact, it has changed a couple
of times in the last couple of weeks.

-Wstringop-overflow will soon be disabled for GCC 11 by default. So,
this patch is unnecessary. I'll probably commit a patch for this next
week. :)

Thanks
--
Gustavo

Re: [PATCH next] Fix the build failed caused by -Wstringop-overflow

[Andy Shevchenko]

On Thu, Nov 30, 2023 at 09:52:42AM -0600, Gustavo A. R. Silva wrote:
> On 11/30/23 04:57, Wenyu Huang wrote:

...

> > Fixes: 89741e7e42f6 ("Makefile: Enable -Wstringop-overflow globally")
> 
> The commit ID is from a patch that's currently in linux-next, which
> does not guarantee it's a stable commit. So, it shouldn't be used
> for any tag in any changelog text. In fact, it has changed a couple
> of times in the last couple of weeks.

I disagree on this in general.

The case in practice I have. I does something in new cycle that broke the
enumeration of some devices. The patch is in the maintainer's tree pending
for the next release (v6.8-rc1). There are I see two options:
- revert patch completely and redo it properly
- add a fix (which is one liner)

Now, what you are suggesting is to drop the Fixes tag on the grounds that
the culprit and the fix are to be in the same release (as we go let's say
with the latter approach). In case that the culprit will be backported
(let's say to satisfy dependencies, as per se it's not a fix), it will
bring a regression and become unnoticed for some time until first reports
will appear. Additional resources would be need for all this.

So, I'm fully in favour of using Fixes tag as it makes clear if we have
some broken changes in the kernel for which the fix is known and exists.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH next] Fix the build failed caused by -Wstringop-overflow

[Andy Shevchenko]

On Thu, Nov 30, 2023 at 07:48:46PM +0200, Andy Shevchenko wrote:
> On Thu, Nov 30, 2023 at 09:52:42AM -0600, Gustavo A. R. Silva wrote:
> > On 11/30/23 04:57, Wenyu Huang wrote:

...

> > > Fixes: 89741e7e42f6 ("Makefile: Enable -Wstringop-overflow globally")
> > 
> > The commit ID is from a patch that's currently in linux-next, which
> > does not guarantee it's a stable commit. So, it shouldn't be used
> > for any tag in any changelog text. In fact, it has changed a couple
> > of times in the last couple of weeks.
> 
> I disagree on this in general.
> 
> The case in practice I have. I does something in new cycle that broke the
> enumeration of some devices. The patch is in the maintainer's tree pending
> for the next release (v6.8-rc1). There are I see two options:
> - revert patch completely and redo it properly
> - add a fix (which is one liner)
> 
> Now, what you are suggesting is to drop the Fixes tag on the grounds that
> the culprit and the fix are to be in the same release (as we go let's say
> with the latter approach). In case that the culprit will be backported
> (let's say to satisfy dependencies, as per se it's not a fix), it will
> bring a regression and become unnoticed for some time until first reports
> will appear. Additional resources would be need for all this.
> 
> So, I'm fully in favour of using Fixes tag as it makes clear if we have
> some broken changes in the kernel for which the fix is known and exists.

On top of that, Fixes tag is not enough to get it to stable. See the rules
on how to submit a material to stable kernels, it's in the documentation.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH next] Fix the build failed caused by -Wstringop-overflow

[Gustavo A. R. Silva]

On 11/30/23 11:50, Andy Shevchenko wrote:
> On Thu, Nov 30, 2023 at 07:48:46PM +0200, Andy Shevchenko wrote:
>> On Thu, Nov 30, 2023 at 09:52:42AM -0600, Gustavo A. R. Silva wrote:
>>> On 11/30/23 04:57, Wenyu Huang wrote:
> 
> ...
> 
>>>> Fixes: 89741e7e42f6 ("Makefile: Enable -Wstringop-overflow globally")
>>>
>>> The commit ID is from a patch that's currently in linux-next, which
>>> does not guarantee it's a stable commit. So, it shouldn't be used
>>> for any tag in any changelog text. In fact, it has changed a couple
>>> of times in the last couple of weeks.
>>
>> I disagree on this in general.
>>
>> The case in practice I have. I does something in new cycle that broke the
>> enumeration of some devices. The patch is in the maintainer's tree pending
>> for the next release (v6.8-rc1). There are I see two options:
>> - revert patch completely and redo it properly
>> - add a fix (which is one liner)
>>
>> Now, what you are suggesting is to drop the Fixes tag on the grounds that
>> the culprit and the fix are to be in the same release (as we go let's say
>> with the latter approach). In case that the culprit will be backported
>> (let's say to satisfy dependencies, as per se it's not a fix), it will
>> bring a regression and become unnoticed for some time until first reports
>> will appear. Additional resources would be need for all this.
>>
>> So, I'm fully in favour of using Fixes tag as it makes clear if we have
>> some broken changes in the kernel for which the fix is known and exists.
> 
> On top of that, Fixes tag is not enough to get it to stable. See the rules
> on how to submit a material to stable kernels, it's in the documentation.

We are talking about different things. I'm talking about commit IDs staying
unchanged (stable commit IDs). That's different to stable kernels. :)

--
Gustavo

Re: [PATCH next] Fix the build failed caused by -Wstringop-overflow

[Andy Shevchenko]

On Thu, Nov 30, 2023 at 12:04:28PM -0600, Gustavo A. R. Silva wrote:
> On 11/30/23 11:50, Andy Shevchenko wrote:
> > On Thu, Nov 30, 2023 at 07:48:46PM +0200, Andy Shevchenko wrote:
> > > On Thu, Nov 30, 2023 at 09:52:42AM -0600, Gustavo A. R. Silva wrote:
> > > > On 11/30/23 04:57, Wenyu Huang wrote:

...

> > On top of that, Fixes tag is not enough to get it to stable. See the rules
> > on how to submit a material to stable kernels, it's in the documentation.
> 
> We are talking about different things. I'm talking about commit IDs staying
> unchanged (stable commit IDs). That's different to stable kernels. :)

I see, but look at my answer to your previous email.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH next] Fix the build failed caused by -Wstringop-overflow

[Gustavo A. R. Silva]

On 11/30/23 12:06, Andy Shevchenko wrote:
> On Thu, Nov 30, 2023 at 12:04:28PM -0600, Gustavo A. R. Silva wrote:
>> On 11/30/23 11:50, Andy Shevchenko wrote:
>>> On Thu, Nov 30, 2023 at 07:48:46PM +0200, Andy Shevchenko wrote:
>>>> On Thu, Nov 30, 2023 at 09:52:42AM -0600, Gustavo A. R. Silva wrote:
>>>>> On 11/30/23 04:57, Wenyu Huang wrote:
> 
> ...
> 
>>> On top of that, Fixes tag is not enough to get it to stable. See the rules
>>> on how to submit a material to stable kernels, it's in the documentation.
>>
>> We are talking about different things. I'm talking about commit IDs staying
>> unchanged (stable commit IDs). That's different to stable kernels. :)
> 
> I see, but look at my answer to your previous email.
> 

Yes, we are on the same page.

Thanks
--
Gustavo

Re: [PATCH next] Fix the build failed caused by -Wstringop-overflow

[Andrew Morton]

On Thu, 30 Nov 2023 12:04:28 -0600 "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote:

> > On top of that, Fixes tag is not enough to get it to stable. See the rules
> > on how to submit a material to stable kernels, it's in the documentation.
> 
> We are talking about different things. I'm talking about commit IDs staying
> unchanged (stable commit IDs). That's different to stable kernels. :)

That's why we use the 

	04448A0E ("wobble the fronnozzle")

format.  Searching for the title is the fallback option.


And I agree with what appears to be everyone else.  Always include the
Fixes: if possible.  So that people don't accidentally cherrypick a
known-to-be-broken patch.

Re: [PATCH next] Fix the build failed caused by -Wstringop-overflow

[Gustavo A. R. Silva]

On 11/30/23 11:48, Andy Shevchenko wrote:
> On Thu, Nov 30, 2023 at 09:52:42AM -0600, Gustavo A. R. Silva wrote:
>> On 11/30/23 04:57, Wenyu Huang wrote:
> 
> ...
> 
>>> Fixes: 89741e7e42f6 ("Makefile: Enable -Wstringop-overflow globally")
>>
>> The commit ID is from a patch that's currently in linux-next, which
>> does not guarantee it's a stable commit. So, it shouldn't be used
>> for any tag in any changelog text. In fact, it has changed a couple
>> of times in the last couple of weeks.
> 
> I disagree on this in general.
> 
> The case in practice I have. I does something in new cycle that broke the
> enumeration of some devices. The patch is in the maintainer's tree pending
> for the next release (v6.8-rc1). There are I see two options:
> - revert patch completely and redo it properly
> - add a fix (which is one liner)
> 
> Now, what you are suggesting is to drop the Fixes tag on the grounds that
> the culprit and the fix are to be in the same release (as we go let's say
> with the latter approach). In case that the culprit will be backported
> (let's say to satisfy dependencies, as per se it's not a fix), it will
> bring a regression and become unnoticed for some time until first reports
> will appear. Additional resources would be need for all this.
> 
> So, I'm fully in favour of using Fixes tag as it makes clear if we have
> some broken changes in the kernel for which the fix is known and exists.

I'm fully in favor, as well. :)

I'm talking about patches in linux-next, exclusively.

--
Gustavo

Re: [PATCH next] Fix the build failed caused by -Wstringop-overflow

[Andy Shevchenko]

On Thu, Nov 30, 2023 at 11:56:29AM -0600, Gustavo A. R. Silva wrote:
> On 11/30/23 11:48, Andy Shevchenko wrote:
> > On Thu, Nov 30, 2023 at 09:52:42AM -0600, Gustavo A. R. Silva wrote:
> > > On 11/30/23 04:57, Wenyu Huang wrote:

...

> > > > Fixes: 89741e7e42f6 ("Makefile: Enable -Wstringop-overflow globally")
> > > 
> > > The commit ID is from a patch that's currently in linux-next, which
> > > does not guarantee it's a stable commit. So, it shouldn't be used
> > > for any tag in any changelog text. In fact, it has changed a couple
> > > of times in the last couple of weeks.
> > 
> > I disagree on this in general.
> > 
> > The case in practice I have. I does something in new cycle that broke the
> > enumeration of some devices. The patch is in the maintainer's tree pending
> > for the next release (v6.8-rc1). There are I see two options:
> > - revert patch completely and redo it properly
> > - add a fix (which is one liner)
> > 
> > Now, what you are suggesting is to drop the Fixes tag on the grounds that
> > the culprit and the fix are to be in the same release (as we go let's say
> > with the latter approach). In case that the culprit will be backported
> > (let's say to satisfy dependencies, as per se it's not a fix), it will
> > bring a regression and become unnoticed for some time until first reports
> > will appear. Additional resources would be need for all this.
> > 
> > So, I'm fully in favour of using Fixes tag as it makes clear if we have
> > some broken changes in the kernel for which the fix is known and exists.
> 
> I'm fully in favor, as well. :)
> 
> I'm talking about patches in linux-next, exclusively.

And I am. And I think it's a good practice independently on the Linux Next
case. The only what you should think about is that the maintainer has to
take care about proper commit ID in case they rebase their tree (spoiler:
they shouldn't in 99.99% of cases).

My "in general" was to make sure we are on the page not specifically related
to _this_ patch.

-- 
With Best Regards,
Andy Shevchenko