[PATCH v19 00/11] Support translated long option names in getopt and argp
Vivien Kraus
vivien@planete-kraus.eu
Sat Dec 6 13:19:07 GMT 2025
Dear Adhemerval Zanella Netto and other glibc developers,
Thank you for your feedback. It is difficult to find the last bits of
bad indentation, however I have set a dir-local variable in emacs to
use tabs as much as possible, so the indentation should be correct in
the future.
I added a dependency on $(gen-locales) for the tests.
Le 01/12/2025 à 21:34, Adhemerval Zanella Netto a écrit :
>> # Test for the glob symbol version that was replaced in glibc 2.27.
>> ifeq ($(have-GLIBC_2.26)$(build-shared),yesyes)
>> tests += \
>> diff --git a/posix/getopt.c b/posix/getopt.c
>> index 66b43850ee..b49dfb643f 100644
>> --- a/posix/getopt.c
>> +++ b/posix/getopt.c
>> @@ -182,6 +182,21 @@ exchange (char **argv, struct _getopt_data *d)
>> d->__last_nonopt = d->optind;
>> }
>>
>> +/* Return 1 iff a translation for opt_name has been found and it
>> + matches the substring from argument, length argument_length.
>> +*/
>> +static const int
>> +match_translated_option_name (char *(*translate) (const char *msgid),
>> + const char *argument, size_t argument_length,
>> + const char *opt_name)
>> +{
>> + const char *translated = opt_name;
>> + if (translate != NULL)
>> + translated = translate (opt_name);
>> + return (!strncmp (translated, argument, argument_length)
>> + && argument_length == strlen (translated));
>> +}
>> +
> I think that we can optimize it to the following:
>
> if (translated[argument_length] != '\0')
> return 0;
> return memcmp (translated, argument, argument_length) == 0;
I’m not sure about that; what if getopt tries to match a very long
argument against a shorter translation? This would index the
translation out of bounds. Am I missing something?
>> + if (strcmp (translated_option_name, pfound->name) != 0)
>> + {
>> + /* Print both names of the option. */
>> + fprintf (stderr,
>> + _("%s: option '%s%s' / '%s%s' doesn't allow an argument\n"),
>> + argv[0], prefix, translated_option_name, prefix, pfound->name);
>> + }
>> + else
>> + {
>> + /* Either the option name is not translated, or its
>> + translation is the same as the option name. */
>> + fprintf (stderr,
>> + _("%s: option '%s%s' doesn't allow an argument\n"),
>> + argv[0], prefix, pfound->name);
>> + }
>
> I think the usual GNU code style is to omit brackets for one-line directive.
I thought the braces were required around a block composed of a
comment and an instruction, but seeing surrounding code, I now
understand.
>> diff --git a/posix/tstgetoptl.c b/posix/tstgetoptl.c
>> [...]
>> +
>> +/* This tests that --colour is accepted as a translation of --color.
>> + This echoes tstgetopt.c, where --colour was an option name alias
>> + for --color, so it had to be listed twice. */
>> +
>> +/* This uses the en_GB locale so that colour means color. As a
>> + special case, we also check that non-translated options have
>> + precedence over translated options, by translated "optional" as
>> + "required". */
>
> Should we also check when a translation is required, but there is no
> suitable option to check for the error paths?
Do you mean: if the user passes --flavour, there is a “flavor” ->
“flavour” en_GB translation, but flavor is not an option recognized by
the program, then getopt should detect an unrecognized option? I
added a test for this.
>> diff --git a/posix/getopt1.c b/posix/getopt1.c
>> [...]
>> +/* FIXME: use pgettext_expr. */
>> +static char *
>> +do_translate (const char *context, const char *msgid)
>> +{
>> + char *full_msgid;
>> + const char *translated = msgid;
>> + int output_length = 0;
>> +
>> + if (context != NULL)
>> + {
>> + output_length = __asprintf (&full_msgid, "%s\004%s", context, msgid);
>> + if (output_length >= 0)
>> + {
>> + translated = __dcgettext (NULL, full_msgid, LC_MESSAGES);
>> + if (strcmp (translated, full_msgid) == 0)
>> + {
>> + /* No translation for this context and message, so drop
>> + the context + ^D prefix. */
>> + translated = msgid;
>> + }
>> + }
>> + /* Otherwise, if memory allocation failed, then we won’t accept
>> + translations. translated remains an alias to msgid. */
>> + free (full_msgid);
>
> Wouldn't this possible return a free pointer as translated? On __dcgettext,
> it calls __dcigettext (NULL, full_msgid, NULL, 0, 0, LC_MESSAGES) and then:
>
> 454 char *
> 455 DCIGETTEXT (const char *domainname, const char *msgid1, const char *msgid2,
> 456 int plural, unsigned long int n, int category)
> 457 #endif
> 458 {
> [...]
> 826 __set_errno (saved_errno);
> 827 return (plural == 0
> 828 ? (char *) msgid1
> 829 /* Use the Germanic plural rule. */
> 830 : n == 1 ? (char *) msgid1 : (char *) msgid2);
> 831 }
> 488 #endif
>
> For the unstranslted case? Or am I missing something here?
It should not happen, because if __dcgettext returns the allocated
full_msgid, then we detect it in the “No translation” case, and
replace it with the non-allocated msgid. In any case, it should only
return the exact full_msgid pointer, or nothing that aliases it. I
agree that it depends a bit too much on the dgettext implementation,
so this revision adds a pointer to allocated data that must be kept
around until we are done with the translation. Do you prefer it that
way?
Best regards,
Vivien
Vivien Kraus (11):
posix: allow getopt_long to match translated option names
posix: let the getopt caller set the translation context
argp: document translated names in --help and --usage
posix: let the getopt caller choose the textdomain for translation
posix: do not allow option name translations for secure programs
posix, argp: Support deprecation of long option name translations
argp: do not display option name translations if __libc_enable_secure
posix: Add getopt_long_collision
posix: Add a script for static validation of getopt_long PO files
Add NEWS entry for getopt_long translated option names
support: Document the correct macro name for (CMDLINE_OPTIONS)
NEWS | 2 +
argp/Makefile | 14 ++
argp/argp-help.c | 76 ++++++-
argp/argp-parse.c | 1 +
argp/tst-argphelp-localized.c | 140 ++++++++++++
argp/tst-argphelp-localized.po | 23 ++
argp/tst-argpusage-localized.c | 80 +++++++
manual/argp.texi | 25 ++-
manual/getopt.texi | 81 ++++++-
posix/Makefile | 39 ++++
posix/Versions | 3 +
posix/bits/getopt_ext.h | 17 ++
posix/check-getopt-translations.pl | 199 ++++++++++++++++++
posix/getopt.c | 171 +++++++++++++--
posix/getopt1.c | 110 +++++++++-
posix/getopt_int.h | 14 +-
.../standalone-multiple-getopt-collisions.po | 45 ++++
posix/tst-check-getopt-translations.sh | 59 ++++++
posix/tst-getopt_long_collision.c | 70 ++++++
posix/tst-getopt_long_collision.po | 22 ++
posix/tstgetoptl.c | 169 +++++++++++++++
posix/tstgetoptl.po | 32 +++
support/test-driver.c | 2 +-
sysdeps/mach/hurd/i386/libc.abilist | 3 +
sysdeps/mach/hurd/x86_64/libc.abilist | 3 +
sysdeps/unix/sysv/linux/aarch64/libc.abilist | 3 +
sysdeps/unix/sysv/linux/alpha/libc.abilist | 3 +
sysdeps/unix/sysv/linux/arc/libc.abilist | 3 +
sysdeps/unix/sysv/linux/arm/be/libc.abilist | 3 +
sysdeps/unix/sysv/linux/arm/le/libc.abilist | 3 +
sysdeps/unix/sysv/linux/csky/libc.abilist | 3 +
sysdeps/unix/sysv/linux/hppa/libc.abilist | 3 +
sysdeps/unix/sysv/linux/i386/libc.abilist | 3 +
.../sysv/linux/loongarch/lp64/libc.abilist | 3 +
.../sysv/linux/m68k/coldfire/libc.abilist | 3 +
.../unix/sysv/linux/m68k/m680x0/libc.abilist | 3 +
.../sysv/linux/microblaze/be/libc.abilist | 3 +
.../sysv/linux/microblaze/le/libc.abilist | 3 +
.../sysv/linux/mips/mips32/fpu/libc.abilist | 3 +
.../sysv/linux/mips/mips32/nofpu/libc.abilist | 3 +
.../sysv/linux/mips/mips64/n32/libc.abilist | 3 +
.../sysv/linux/mips/mips64/n64/libc.abilist | 3 +
sysdeps/unix/sysv/linux/or1k/libc.abilist | 3 +
.../linux/powerpc/powerpc32/fpu/libc.abilist | 3 +
.../powerpc/powerpc32/nofpu/libc.abilist | 3 +
.../linux/powerpc/powerpc64/be/libc.abilist | 3 +
.../linux/powerpc/powerpc64/le/libc.abilist | 3 +
.../unix/sysv/linux/riscv/rv32/libc.abilist | 3 +
.../unix/sysv/linux/riscv/rv64/libc.abilist | 3 +
.../unix/sysv/linux/s390/s390-32/libc.abilist | 3 +
.../unix/sysv/linux/s390/s390-64/libc.abilist | 3 +
sysdeps/unix/sysv/linux/sh/be/libc.abilist | 3 +
sysdeps/unix/sysv/linux/sh/le/libc.abilist | 3 +
.../sysv/linux/sparc/sparc32/libc.abilist | 3 +
.../sysv/linux/sparc/sparc64/libc.abilist | 3 +
.../unix/sysv/linux/x86_64/64/libc.abilist | 3 +
.../unix/sysv/linux/x86_64/x32/libc.abilist | 3 +
57 files changed, 1459 insertions(+), 37 deletions(-)
create mode 100644 argp/tst-argphelp-localized.c
create mode 100644 argp/tst-argphelp-localized.po
create mode 100644 argp/tst-argpusage-localized.c
create mode 100644 posix/check-getopt-translations.pl
create mode 100644 posix/standalone-multiple-getopt-collisions.po
create mode 100644 posix/tst-check-getopt-translations.sh
create mode 100644 posix/tst-getopt_long_collision.c
create mode 100644 posix/tst-getopt_long_collision.po
create mode 100644 posix/tstgetoptl.c
create mode 100644 posix/tstgetoptl.po
base-commit: 866fa41ef8521ce94ffdacfd6f1f67737899d5c9
--
2.34.1
More information about the Libc-alpha
mailing list