[PATCH] Don't use locale functions when libintl header isn't included.

[PATCH] Don't use locale functions when libintl header isn't included.

[Érico Nogueira]

debuginfod.cxx used the bindtextdomain() and textdomain() functions
despite not including any translated output. These functions were also
used without including the libintl.h header.

Signed-off-by: Érico Rolim <erico.erc@gmail.com>
---
 debuginfod/debuginfod.cxx | 2 --
 1 file changed, 2 deletions(-)

diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 2b68ff1f..bb95aefb 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -3092,8 +3092,6 @@ int
 main (int argc, char *argv[])
 {
   (void) setlocale (LC_ALL, "");
-  (void) bindtextdomain (PACKAGE_TARNAME, LOCALEDIR);
-  (void) textdomain (PACKAGE_TARNAME);
 
   /* Tell the library which version we are expecting.  */
   elf_version (EV_CURRENT);
-- 
2.29.0

Re: [PATCH] Don't use locale functions when libintl header isn't included.

[Mark Wielaard]

Hi Érico,

On Mon, 2020-10-26 at 01:11 -0300, Érico Nogueira via Elfutils-devel wrote:
> debuginfod.cxx used the bindtextdomain() and textdomain() functions
> despite not including any translated output. These functions were also
> used without including the libintl.h header.

debuginfod doesn't directly use any translated output, but it links
against libeu.a which provides the print_version function, which does
use translated output. So maybe the correct fix is to #include
<libintl.h> instead?

Cheers,

Mark

Re: [PATCH] Don't use locale functions when libintl header isn't included.

[Érico Nogueira]

On Mon Oct 26, 2020 at 4:46 PM -03, Mark Wielaard wrote:
> Hi Érico,
>
> On Mon, 2020-10-26 at 01:11 -0300, Érico Nogueira via Elfutils-devel
> wrote:
> > debuginfod.cxx used the bindtextdomain() and textdomain() functions
> > despite not including any translated output. These functions were also
> > used without including the libintl.h header.
>
> debuginfod doesn't directly use any translated output, but it links
> against libeu.a which provides the print_version function, which does
> use translated output. So maybe the correct fix is to #include
> <libintl.h> instead?
>
> Cheers,
>
> Mark

Hi Mark,

I had talked with fche on #elfutils, and it was suggested that I could
remove the locale functions from debuginfod.cxx. If you think it makes
more sense to simply include the header (which is what I did initially
for my own build), I guess I should remove the comment about C++ support
as well, right? Or modify it to something else, at least. Would you have
any suggestions for that?

Thanks,
Érico

Re: [PATCH] Don't use locale functions when libintl header isn't included.

[Mark Wielaard]

On Mon, 2020-10-26 at 15:53 -0300, Érico Nogueira via Elfutils-devel
wrote:
> I had talked with fche on #elfutils, and it was suggested that I could
> remove the locale functions from debuginfod.cxx. If you think it makes
> more sense to simply include the header (which is what I did initially
> for my own build), I guess I should remove the comment about C++ support
> as well, right? Or modify it to something else, at least. Would you have
> any suggestions for that?

Aha, I hadn't seen that before, you mean this line in debuginfod.cxx:

  // #include <libintl.h> // not until it supports C++ << better

I think it is nice if all elfutils tools would use the same
print_version function so --version always produces the same thing.

I think that is the only "real" thing in debuginfod.cxx that uses
localization and I don't believe anybody actually submitted a
translation of that notice.

If so (you'll have to double check) and it makes things easier/cleaner
for debuginfod then you could remove the gettext call from
print_version and remove the textdomain calls (please also remove the
commented out include). Especially if Frank (fche) agrees.

But if it is all the same, then I would simply add the include for
libintl.h and remove or update the comment to say this is just form
calling textdomain for some C helper functions, but isn't used in the
C++ code (yet).

Cheers,

Mark

Re: [PATCH] Don't use locale functions when libintl header isn't included.

[Frank Ch. Eigler]

Hi -

> On Mon, 2020-10-26 at 01:11 -0300, Érico Nogueira via Elfutils-devel wrote:
> > debuginfod.cxx used the bindtextdomain() and textdomain() functions
> > despite not including any translated output. These functions were also
> > used without including the libintl.h header.
> 
> debuginfod doesn't directly use any translated output, but it links
> against libeu.a which provides the print_version function, which does
> use translated output. So maybe the correct fix is to #include
> <libintl.h> instead?

I think their concern may be that they don't have any libintl.h or
related functions in the musl world.

- FChE

Re: [PATCH] Don't use locale functions when libintl header isn't included.

[Érico Nogueira]

On Mon Oct 26, 2020 at 3:22 PM -03, Frank Ch. Eigler wrote:
> Hi -
>
> > On Mon, 2020-10-26 at 01:11 -0300, Érico Nogueira via Elfutils-devel wrote:
> > > debuginfod.cxx used the bindtextdomain() and textdomain() functions
> > > despite not including any translated output. These functions were also
> > > used without including the libintl.h header.
> > 
> > debuginfod doesn't directly use any translated output, but it links
> > against libeu.a which provides the print_version function, which does
> > use translated output. So maybe the correct fix is to #include
> > <libintl.h> instead?
>
> I think their concern may be that they don't have any libintl.h or
> related functions in the musl world.
>
> - FChE

No, musl does provide a libintl.h header and the necessary
implementations. My changes to debuginfod.cxx were only necessary
because the file was using functions from libintl.h even though the
header itself wasn't included. Following your comment on IRC, I will
make a new patch that just includes libintl.h.

Cheers,
Érico