Patchwork Fix uninitialized variable in assert_perror (bug 22761)

login
register
mail settings
Submitter Andreas Schwab
Date Jan. 31, 2018, 9:55 a.m.
Message ID <mvma7wuic7z.fsf@suse.de>
Download mbox | patch
Permalink /patch/25702/
State New
Headers show

Comments

Andreas Schwab - Jan. 31, 2018, 9:55 a.m.
I wasn't able to get assert_perror to misbehave in an observable way.

	[BZ #22761]
	* assert/assert-perr.c (__assert_perror_fail): Append %n to format
	string.
---
 assert/assert-perr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Florian Weimer - Jan. 31, 2018, 10:45 a.m.
On 01/31/2018 10:55 AM, Andreas Schwab wrote:
> I wasn't able to get assert_perror to misbehave in an observable way.
> 
> 	[BZ #22761]
> 	* assert/assert-perr.c (__assert_perror_fail): Append %n to format
> 	string.

The patch looks good to me.  I'm not sure if it should go in so shortly 
before the release.

Thanks,
Florian
Dmitry Levin - Jan. 31, 2018, 10:54 a.m.
On Wed, Jan 31, 2018 at 10:55:44AM +0100, Andreas Schwab wrote:
> I wasn't able to get assert_perror to misbehave in an observable way.
> 
> 	[BZ #22761]
> 	* assert/assert-perr.c (__assert_perror_fail): Append %n to format
> 	string.
> ---
>  assert/assert-perr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/assert/assert-perr.c b/assert/assert-perr.c
> index 002a5e8708..4b4fe883fd 100644
> --- a/assert/assert-perr.c
> +++ b/assert/assert-perr.c
> @@ -32,7 +32,7 @@ __assert_perror_fail (int errnum,
>    char errbuf[1024];
>  
>    char *e = __strerror_r (errnum, errbuf, sizeof errbuf);
> -  __assert_fail_base (_("%s%s%s:%u: %s%sUnexpected error: %s.\n"),
> +  __assert_fail_base (_("%s%s%s:%u: %s%sUnexpected error: %s.\n%n"),
>  		      e, file, line, function);
>  }
>  libc_hidden_def (__assert_perror_fail)

The patch is obviously correct but it breaks translations.
How do we handle this before the release?
Carlos O'Donell - Jan. 31, 2018, 11:42 p.m.
On 01/31/2018 02:54 AM, Dmitry V. Levin wrote:
> On Wed, Jan 31, 2018 at 10:55:44AM +0100, Andreas Schwab wrote:
>> I wasn't able to get assert_perror to misbehave in an observable way.
>>
>> 	[BZ #22761]
>> 	* assert/assert-perr.c (__assert_perror_fail): Append %n to format
>> 	string.
>> ---
>>  assert/assert-perr.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/assert/assert-perr.c b/assert/assert-perr.c
>> index 002a5e8708..4b4fe883fd 100644
>> --- a/assert/assert-perr.c
>> +++ b/assert/assert-perr.c
>> @@ -32,7 +32,7 @@ __assert_perror_fail (int errnum,
>>    char errbuf[1024];
>>  
>>    char *e = __strerror_r (errnum, errbuf, sizeof errbuf);
>> -  __assert_fail_base (_("%s%s%s:%u: %s%sUnexpected error: %s.\n"),
>> +  __assert_fail_base (_("%s%s%s:%u: %s%sUnexpected error: %s.\n%n"),
>>  		      e, file, line, function);
>>  }
>>  libc_hidden_def (__assert_perror_fail)
> 
> The patch is obviously correct but it breaks translations.
> How do we handle this before the release?

The bug in question has been around for 7 years.

I think this bug fix should be delayed until 2.28.

Fixing bugs trumps having translations.

The translations attempt to be as accurate as we can make them,
but slowly the release branch and master drift away from the
translation strings as things are updated to fix bugs.

At each release boundary we bring the translations back into
line with what the translation team provides.

Patch

diff --git a/assert/assert-perr.c b/assert/assert-perr.c
index 002a5e8708..4b4fe883fd 100644
--- a/assert/assert-perr.c
+++ b/assert/assert-perr.c
@@ -32,7 +32,7 @@  __assert_perror_fail (int errnum,
   char errbuf[1024];
 
   char *e = __strerror_r (errnum, errbuf, sizeof errbuf);
-  __assert_fail_base (_("%s%s%s:%u: %s%sUnexpected error: %s.\n"),
+  __assert_fail_base (_("%s%s%s:%u: %s%sUnexpected error: %s.\n%n"),
 		      e, file, line, function);
 }
 libc_hidden_def (__assert_perror_fail)