Hi, Eli Zaretskii <e...@gnu.org> writes:
>> Whereas with the Gnulib 'error' module, there is a conflict between the >> two global function definitions (with 'T' linkage) in install-info.c and >> in error.c *always*. >> >> > The simplest way to fix this problem would probably be to rename the >> > "error" >> > function in install-info.c. >> >> Yes, or make it 'static' (like Arsen suggested). > > Shouldn't we report this to the GCC folks? It could be a bug in lto, > no? I mean, 'error' is not a symbol that applications cannot use, and > if an application defines a function by that name, it should not be > imported from the standard library. Right? I believe this would make them part of the same program. On top of that, Gnulib is pulling in error anyway: $ nm ./gnulib/lib/libgnu.a | grep error U error $ nm install-info.o ../gnulib/lib/libgnu.a |& grep '\<error\>' 00000000 T error U error My guess is that libgnu_a-xalloc-die.o (the file emitting the U error symbol) includes gnulib/lib/error.h, GCC records that declaration (through it's use in xalloc_die), and then detects a mismatch with the one emitted by install-info.o (the T error symbol) and hence warns. I imagine this would result is some very strange runtime failures if anyone ever observed install-info hit an xalloc_die condition. As a test, building on musl (which lacks the error API, for some reason) causes the executable to be compiled with the install-info error, rather than the Gnulib one, demonstrating why this warning happens. Attempting to --whole-archive link on that platform grants us: $ x86_64-linux-musl-gcc -o ginstall-info install-info.o \ -Wl,--whole-archive ../gnulib/lib/libgnu.a -Wl,--no-whole-archive /usr/libexec/gcc/x86_64-linux-musl/ld: ../gnulib/lib/libgnu.a(libgnu_a-error.o): in function `error': error.c:(.text+0xe0): multiple definition of `error'; install-info.o:install-info.c:(.text+0x4a0): first defined here collect2: error: ld returned 1 exit status I imagine a similar thing would happen with a static glibc link. Renaming the functions or adding ``static'' elides this issue. So, GCC appears to be doing the right thing. Since I went through the process of making all the symbols in that file (besides main) local, here's the patch that does that, though it's based on a not-particularly-clean head (so, ChangeLog might conflict):
From 65b7657bfc0bc84178c95211690be94c767d725f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arsen=20Arsenovi=C4=87?= <ar...@aarsen.me> Date: Thu, 8 Dec 2022 09:52:13 +0100 Subject: [PATCH] install-info: Make local symbols static * install-info/install-info.c: Make local symbols static. --- ChangeLog | 5 +++ install-info/install-info.c | 62 ++++++++++++++++++------------------- 2 files changed, 36 insertions(+), 31 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9bfba11abb..98c2764b99 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2022-12-08 Arsen Arsenović <ar...@aarsen.me> + + install-info: Make local symbols static. + * install-info/install-info.c: Make local symbols static. + 2022-12-01 Arsen Arsenović <ar...@aarsen.me> Re-enable copyable anchors in HTML output diff --git a/install-info/install-info.c b/install-info/install-info.c index 8950288f6b..761cbfb4d4 100644 --- a/install-info/install-info.c +++ b/install-info/install-info.c @@ -28,11 +28,11 @@ static char *default_section = NULL; struct spec_entry; struct spec_section; -struct line_data *findlines (char *data, int size, int *nlinesp); -void insert_entry_here (struct spec_entry *entry, int line_number, - struct line_data *dir_lines, int n_entries); -int compare_section_names (const void *s1, const void *s2); -int compare_entries_text (const void *e1, const void *e2); +static struct line_data *findlines (char *data, int size, int *nlinesp); +static void insert_entry_here (struct spec_entry *entry, int line_number, + struct line_data *dir_lines, int n_entries); +static int compare_section_names (const void *s1, const void *s2); +static int compare_entries_text (const void *e1, const void *e2); /* Data structures. */ @@ -136,7 +136,7 @@ struct menu_section /* This table defines all the long-named options, says whether they use an argument, and maps them into equivalent single-letter options. */ -struct option longopts[] = +static struct option longopts[] = { { "add-once", no_argument, NULL, '1'}, { "align", required_argument, NULL, 'A'}, @@ -172,39 +172,39 @@ struct option longopts[] = { 0 } }; -regex_t *psecreg = NULL; +static regex_t *psecreg = NULL; /* Nonzero means that the name specified for the Info file will be used (without removing .gz, .info extension or leading path) to match the entries that must be removed. */ -int remove_exactly = 0; +static int remove_exactly = 0; /* Nonzero means that sections that don't have entries in them will be deleted. */ -int remove_empty_sections = 1; +static int remove_empty_sections = 1; /* Nonzero means that new Info entries into the DIR file go into all sections that match with --section-regex or --section. Zero means that new entries go into only the first section that matches. */ -int add_entries_into_all_matching_sections = 1; +static int add_entries_into_all_matching_sections = 1; /* Nonzero means we do not replace same-named info entries. */ -int keep_old_flag = 0; +static int keep_old_flag = 0; /* Nonzero means --test was specified, to inhibit updating the dir file. */ -int chicken_flag = 0; +static int chicken_flag = 0; /* Zero means that entries will not be formatted when they are either added or replaced. */ -int indent_flag = 1; +static int indent_flag = 1; /* Zero means that new sections will be added at the end of the DIR file. */ -int order_new_sections_alphabetically_flag = 1; +static int order_new_sections_alphabetically_flag = 1; /* Error message functions. */ -void +static void vdiag (const char *fmt, const char *diagtype, va_list ap) { fprintf (stderr, "%s: ", progname); @@ -214,7 +214,7 @@ vdiag (const char *fmt, const char *diagtype, va_list ap) putc ('\n', stderr); } -void +static void error (const char *fmt, ...) { va_list ap; @@ -225,7 +225,7 @@ error (const char *fmt, ...) } /* VARARGS1 */ -void +static void warning (const char *fmt, ...) { va_list ap; @@ -237,7 +237,7 @@ warning (const char *fmt, ...) /* Print error message and exit. */ -void +static void fatal (const char *fmt, ...) { va_list ap; @@ -250,7 +250,7 @@ fatal (const char *fmt, ...) /* Return a newly-allocated string whose contents concatenate those of S1, S2, S3. */ -char * +static char * concat (const char *s1, const char *s2, const char *s3) { int len1 = strlen (s1), len2 = strlen (s2), len3 = strlen (s3); @@ -267,7 +267,7 @@ concat (const char *s1, const char *s2, const char *s3) /* Return a string containing SIZE characters copied from starting at STRING. */ -char * +static char * copy_string (const char *string, int size) { int i; @@ -280,7 +280,7 @@ copy_string (const char *string, int size) /* Print fatal error message based on errno, with file name NAME. */ -void +static void pfatal_with_name (const char *name) { /* Empty files don't set errno, so we get something like @@ -348,7 +348,7 @@ menu_line_equal (char *line1, int len1, char *line2, int len2) /* Given the full text of a menu entry, null terminated, return just the menu item name (copied). */ -char * +static char * extract_menu_item_name (char *item_text) { char *p; @@ -487,7 +487,7 @@ menu_item_equal (const char *item, char term_char, const char *name) -void +static void suggest_asking_for_help (void) { fprintf (stderr, _("\tTry `%s --help' for a complete list of options.\n"), @@ -495,7 +495,7 @@ suggest_asking_for_help (void) exit (EXIT_FAILURE); } -void +static void print_help (void) { printf (_("Usage: %s [OPTION]... [INFO-FILE [DIR-FILE]]\n"), progname); @@ -641,7 +641,7 @@ The first time you invoke Info you start off looking at this node.\n\ Return the actual name of the file we tried to open in OPENED_FILENAME and the compress program to (de)compress it in COMPRESSION_PROGRAM. */ -FILE * +static FILE * open_possibly_compressed_file (char *filename, void (*create_callback) (char *), char **opened_filename, char **compression_program) @@ -864,7 +864,7 @@ determine_file_type: into COMPRESSION_PROGRAM (if that is non-NULL). If trouble, return a null pointer. */ -char * +static char * readfile (char *filename, int *sizep, void (*create_callback) (char *), char **opened_filename, char **compression_program) @@ -1054,7 +1054,7 @@ output_dirfile (char *dirfile, int dir_nlines, struct line_data *dir_lines, is recorded in next->entry_sections and next->entry_sections_tail, where next is the new entry. Return the number of entries to add from this file. */ -int +static int parse_input (const struct line_data *lines, int nlines, struct spec_section **sections, struct spec_entry **entries, int delete_flag) @@ -1286,7 +1286,7 @@ parse_dir_file (struct line_data *lines, int nlines, struct node **nodes) that matches NAME. If such an entry is found, flag the entry for deletion later on. */ -int +static int mark_entry_for_deletion (struct line_data *lines, int nlines, char *name) { int something_deleted = 0; @@ -1671,7 +1671,7 @@ reformat_new_entries (struct spec_entry *entries, int calign_cli, int align_cli, NAME is the basename of the Info file being installed. The idea here is that there was a --name on the command-line and we need to put the basename in the empty parentheses. */ -void +static void add_missing_basenames (struct spec_entry *entries, char *name) { struct spec_entry *entry; @@ -1706,7 +1706,7 @@ add_missing_basenames (struct spec_entry *entries, char *name) /* Add NAME to the start of any entry in ENTRIES that is missing a name component. If NAME doesn't start with `*', it is formatted to look like an Info entry. */ -void +static void add_missing_names (struct spec_entry *entries, char *name) { struct spec_entry *entry; @@ -1746,7 +1746,7 @@ add_missing_names (struct spec_entry *entries, char *name) /* Append DESC to every entry in ENTRIES that needs it. */ -void +static void add_missing_descriptions (struct spec_entry *entries, char *desc) { struct spec_entry *entry; -- 2.38.1
Have a lovely day. -- Arsen Arsenović
signature.asc
Description: PGP signature