diff mbox series

[v2,2/2] assert: Remove the use of %n from __assert_fail_base (BZ #32456)

Message ID 20241230193700.2851662-2-adhemerval.zanella@linaro.org
State Accepted
Commit 6f0ea84f17581d13ad668adbc181c37141d389b8
Headers show
Series [v2,1/2] Translations: Regenerate libc.pot | expand

Commit Message

Adhemerval Zanella Netto Dec. 30, 2024, 7:36 p.m. UTC 
The require size for mmap can be inferred from __vasprintf return
value.  It also fixes tst-assert-2 when building with --enable-fortify,
where even if the format is not translated, __readonly_area fails
because malloc can not be used.

Checked on aarch64-linux-gnu.
---
 assert/assert-perr.c |  2 +-
 assert/assert.c      | 27 ++++++++++-----------------
 po/libc.pot          | 12 ++++--------
 3 files changed, 15 insertions(+), 26 deletions(-)

Comments

Florian Weimer Dec. 30, 2024, 7:55 p.m. UTC | #1 
* Adhemerval Zanella:

> diff --git a/po/libc.pot b/po/libc.pot
> index d66bd04122..c8193f53c7 100644
> --- a/po/libc.pot
> +++ b/po/libc.pot
> @@ -6,7 +6,7 @@
>  msgid ""
>  msgstr ""
>  "Project-Id-Version: libc 2.40.9000\n"
> -"POT-Creation-Date: 2024-12-30 16:29-0300\n"
> +"POT-Creation-Date: 2024-12-30 16:34-0300\n"
>  "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
>  "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
>  "Language-Team: LANGUAGE <LL@li.org>\n"
> @@ -102,16 +102,12 @@ msgstr ""
>  
>  #: assert/assert-perr.c:35
>  #, c-format
> -msgid ""
> -"%s%s%s:%u: %s%sUnexpected error: %s.\n"
> -"%n"
> +msgid "%s%s%s:%u: %s%sUnexpected error: %s.\n"
>  msgstr ""
>  
> -#: assert/assert.c:132
> +#: assert/assert.c:125
>  #, c-format
> -msgid ""
> -"%s%s%s:%u: %s%sAssertion `%s' failed.\n"
> -"%n"
> +msgid "%s%s%s:%u: %s%sAssertion `%s' failed.\n"
>  msgstr ""
>  
>  #: catgets/gencat.c:111

I would have expect po/*.po file updates, too.

Hmm, maybe I'm wrong, though, and there won't be any crashes, just
missing translations.  And if no one notices, we could remove gettext
from assert in glibc 2.42. 8-)

Thanks,
Florian
Adhemerval Zanella Netto Dec. 30, 2024, 8 p.m. UTC | #2 
On 30/12/24 16:55, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> diff --git a/po/libc.pot b/po/libc.pot
>> index d66bd04122..c8193f53c7 100644
>> --- a/po/libc.pot
>> +++ b/po/libc.pot
>> @@ -6,7 +6,7 @@
>>  msgid ""
>>  msgstr ""
>>  "Project-Id-Version: libc 2.40.9000\n"
>> -"POT-Creation-Date: 2024-12-30 16:29-0300\n"
>> +"POT-Creation-Date: 2024-12-30 16:34-0300\n"
>>  "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
>>  "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
>>  "Language-Team: LANGUAGE <LL@li.org>\n"
>> @@ -102,16 +102,12 @@ msgstr ""
>>  
>>  #: assert/assert-perr.c:35
>>  #, c-format
>> -msgid ""
>> -"%s%s%s:%u: %s%sUnexpected error: %s.\n"
>> -"%n"
>> +msgid "%s%s%s:%u: %s%sUnexpected error: %s.\n"
>>  msgstr ""
>>  
>> -#: assert/assert.c:132
>> +#: assert/assert.c:125
>>  #, c-format
>> -msgid ""
>> -"%s%s%s:%u: %s%sAssertion `%s' failed.\n"
>> -"%n"
>> +msgid "%s%s%s:%u: %s%sAssertion `%s' failed.\n"
>>  msgstr ""
>>  
>>  #: catgets/gencat.c:111
> 
> I would have expect po/*.po file updates, too.

These are from translation teams and current instruction is to not change
it locally [1].  

[1] https://sourceware.org/glibc/wiki/Regeneration

> 
> Hmm, maybe I'm wrong, though, and there won't be any crashes, just
> missing translations.  And if no one notices, we could remove gettext
> from assert in glibc 2.42. 8-)

That's my understanding, without a translation the _(...) just return
the input string.

> 
> Thanks,
> Florian
>
Florian Weimer Jan. 2, 2025, 9:44 a.m. UTC | #3 
* Adhemerval Zanella:

> The require size for mmap can be inferred from __vasprintf return
> value.  It also fixes tst-assert-2 when building with --enable-fortify,
> where even if the format is not translated, __readonly_area fails
> because malloc can not be used.
>
> Checked on aarch64-linux-gnu.

This patch looks okay to me.

Reviewed-by: Florian Weimer <fweimer@redhat.com>

Thanks,
Florian
Maciej W. Rozycki Jan. 3, 2025, 4:33 a.m. UTC | #4 
On Mon, 30 Dec 2024, Adhemerval Zanella Netto wrote:

> >> diff --git a/po/libc.pot b/po/libc.pot
> >> index d66bd04122..c8193f53c7 100644
> >> --- a/po/libc.pot
> >> +++ b/po/libc.pot
> >> @@ -6,7 +6,7 @@
> >>  msgid ""
> >>  msgstr ""
> >>  "Project-Id-Version: libc 2.40.9000\n"
> >> -"POT-Creation-Date: 2024-12-30 16:29-0300\n"
> >> +"POT-Creation-Date: 2024-12-30 16:34-0300\n"
> >>  "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
> >>  "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
> >>  "Language-Team: LANGUAGE <LL@li.org>\n"
> >> @@ -102,16 +102,12 @@ msgstr ""
> >>  
> >>  #: assert/assert-perr.c:35
> >>  #, c-format
> >> -msgid ""
> >> -"%s%s%s:%u: %s%sUnexpected error: %s.\n"
> >> -"%n"
> >> +msgid "%s%s%s:%u: %s%sUnexpected error: %s.\n"
> >>  msgstr ""
> >>  
> >> -#: assert/assert.c:132
> >> +#: assert/assert.c:125
> >>  #, c-format
> >> -msgid ""
> >> -"%s%s%s:%u: %s%sAssertion `%s' failed.\n"
> >> -"%n"
> >> +msgid "%s%s%s:%u: %s%sAssertion `%s' failed.\n"
> >>  msgstr ""
> >>  
> >>  #: catgets/gencat.c:111
> > 
> > I would have expect po/*.po file updates, too.
> 
> These are from translation teams and current instruction is to not change
> it locally [1].  
> 
> [1] https://sourceware.org/glibc/wiki/Regeneration

 As per Andreas's note perhaps it does not matter in this particular case, 
but overall ISTM that such a change ought to really be done in two steps:

1. Remove "%n" from the reference (English) message string and rather than 
   deleting the corresponding pointer argument *replace* it with a pointer 
   to a dummy otherwise unused variable.

2. Only remove the dummy variable and the referring pointer argument once 
   all the translations have been regenerated, perhaps after a release has 
   been made.

 FWIW,

  Maciej
diff mbox series

Patch

diff --git a/assert/assert-perr.c b/assert/assert-perr.c
index 0010cbc278..8a03620b4c 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%n"),
+  __assert_fail_base (_("%s%s%s:%u: %s%sUnexpected error: %s.\n"),
 		      e, file, line, function);
 }
 libc_hidden_def (__assert_perror_fail)
diff --git a/assert/assert.c b/assert/assert.c
index 3a16f85394..3834ddd864 100644
--- a/assert/assert.c
+++ b/assert/assert.c
@@ -15,24 +15,17 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <assert.h>
-#include <atomic.h>
+#include <intprops.h>
 #include <ldsodefs.h>
 #include <libintl.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <sysdep.h>
-#include <unistd.h>
-#include <sys/mman.h>
+#include <libio/iolibio.h>
 #include <setvmaname.h>
 #include <sys/uio.h>
-#include <intprops.h>
+#include <unistd.h>
 
 
 extern const char *__progname;
 
-#include <wchar.h>
-#include <libio/iolibio.h>
 #define fflush(s) _IO_fflush (s)
 
 /* This function, when passed a string containing an asserted
@@ -56,12 +49,12 @@  __assert_fail_base (const char *fmt, const char *assertion, const char *file,
   FATAL_PREPARE;
 #endif
 
-  int total;
-  if (__asprintf (&str, fmt,
-		  __progname, __progname[0] ? ": " : "",
-		  file, line,
-		  function ? function : "", function ? ": " : "",
-		  assertion, &total) >= 0)
+  int total = __asprintf (&str, fmt,
+			  __progname, __progname[0] ? ": " : "",
+			  file, line,
+			  function ? function : "", function ? ": " : "",
+			  assertion);
+  if (total >= 0)
     {
       /* Print the message.  */
       (void) __fxprintf (NULL, "%s", str);
@@ -129,6 +122,6 @@  void
 __assert_fail (const char *assertion, const char *file, unsigned int line,
 	       const char *function)
 {
-  __assert_fail_base (_("%s%s%s:%u: %s%sAssertion `%s' failed.\n%n"),
+  __assert_fail_base (_("%s%s%s:%u: %s%sAssertion `%s' failed.\n"),
 		      assertion, file, line, function);
 }
diff --git a/po/libc.pot b/po/libc.pot
index d66bd04122..c8193f53c7 100644
--- a/po/libc.pot
+++ b/po/libc.pot
@@ -6,7 +6,7 @@ 
 msgid ""
 msgstr ""
 "Project-Id-Version: libc 2.40.9000\n"
-"POT-Creation-Date: 2024-12-30 16:29-0300\n"
+"POT-Creation-Date: 2024-12-30 16:34-0300\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
@@ -102,16 +102,12 @@  msgstr ""
 
 #: assert/assert-perr.c:35
 #, c-format
-msgid ""
-"%s%s%s:%u: %s%sUnexpected error: %s.\n"
-"%n"
+msgid "%s%s%s:%u: %s%sUnexpected error: %s.\n"
 msgstr ""
 
-#: assert/assert.c:132
+#: assert/assert.c:125
 #, c-format
-msgid ""
-"%s%s%s:%u: %s%sAssertion `%s' failed.\n"
-"%n"
+msgid "%s%s%s:%u: %s%sAssertion `%s' failed.\n"
 msgstr ""
 
 #: catgets/gencat.c:111