unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [patch] ldconfig: trace origin paths with -v
@ 2020-02-19 17:50 DJ Delorie
  2020-02-19 18:00 ` Carlos O'Donell
  2020-02-19 18:09 ` Joseph Myers
  0 siblings, 2 replies; 6+ messages in thread
From: DJ Delorie @ 2020-02-19 17:50 UTC (permalink / raw)
  To: libc-alpha


From 74548bffe27d8c37c78b25164b740dd591f681a9 Mon Sep 17 00:00:00 2001
From: DJ Delorie <dj@redhat.com>
Date: Wed, 19 Feb 2020 12:31:38 -0500
Subject: ldconfig: trace origin paths with -v

With this patch, -v turns on a "from" trace for each directory
searched, that tells you WHY that directory is being searched -
is it a builtin, from the command line, or from some config file?

diff --git a/elf/ldconfig.c b/elf/ldconfig.c
index 681ed78496..19ed04cf4d 100644
--- a/elf/ldconfig.c
+++ b/elf/ldconfig.c
@@ -79,6 +79,8 @@ struct dir_entry
   int flag;
   ino64_t ino;
   dev_t dev;
+  const char *from_file;
+  int from_line;
   struct dir_entry *next;
 };
 
@@ -344,7 +346,12 @@ add_single_dir (struct dir_entry *entry, int verbose)
       if (ptr->ino == entry->ino && ptr->dev == entry->dev)
 	{
 	  if (opt_verbose && verbose)
-	    error (0, 0, _("Path `%s' given more than once"), entry->path);
+	    {
+	      error (0, 0, _("Path `%s' given more than once"), entry->path);
+	      fprintf (stderr, "(from %s:%d and %s:%d)\n",
+		       entry->from_file, entry->from_line,
+		       ptr->from_file, ptr->from_line);
+	    }
 	  /* Use the newer information.  */
 	  ptr->flag = entry->flag;
 	  free (entry->path);
@@ -363,12 +370,15 @@ add_single_dir (struct dir_entry *entry, int verbose)
 
 /* Add one directory to the list of directories to process.  */
 static void
-add_dir (const char *line)
+add_dir_1 (const char *line, const char *from_file, int from_line)
 {
   unsigned int i;
   struct dir_entry *entry = xmalloc (sizeof (struct dir_entry));
   entry->next = NULL;
 
+  entry->from_file = strdup (from_file);
+  entry->from_line = from_line;
+
   /* Search for an '=' sign.  */
   entry->path = xstrdup (line);
   char *equal_sign = strchr (entry->path, '=');
@@ -428,6 +438,11 @@ add_dir (const char *line)
     free (path);
 }
 
+static void
+add_dir (const char *line)
+{
+  add_dir_1 (line, "<builtin>", 0);
+}
 
 static int
 chroot_stat (const char *real_path, const char *path, struct stat64 *st)
@@ -672,9 +687,10 @@ search_dir (const struct dir_entry *entry)
   if (opt_verbose)
     {
       if (hwcap != 0)
-	printf ("%s: (hwcap: %#.16" PRIx64 ")\n", entry->path, hwcap);
+	printf ("%s: (hwcap: %#.16" PRIx64 ")", entry->path, hwcap);
       else
-	printf ("%s:\n", entry->path);
+	printf ("%s:", entry->path);
+      printf (" (from %s:%d)\n", entry->from_file, entry->from_line);
     }
 
   char *dir_name;
@@ -815,6 +831,8 @@ search_dir (const struct dir_entry *entry)
 	  struct dir_entry *new_entry;
 
 	  new_entry = xmalloc (sizeof (struct dir_entry));
+	  new_entry->from_file = entry->from_file;
+	  new_entry->from_line = entry->from_line;
 	  new_entry->path = xstrdup (file_name);
 	  new_entry->flag = entry->flag;
 	  new_entry->next = NULL;
@@ -1175,7 +1193,7 @@ Warning: ignoring configuration file that cannot be opened: %s"),
 	    }
 	}
       else
-	add_dir (cp);
+	add_dir_1 (cp, filename, lineno);
     }
   while (!feof_unlocked (file));
 
@@ -1283,7 +1301,7 @@ main (int argc, char **argv)
 		 _("relative path `%s' used to build cache"),
 		 argv[i]);
 	else
-	  add_dir (argv[i]);
+	  add_dir_1 (argv[i], "<cmdline>", 0);
     }
 
   /* The last entry in hwcap_extra is reserved for the "tls" pseudo-hwcap which


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [patch] ldconfig: trace origin paths with -v
  2020-02-19 17:50 [patch] ldconfig: trace origin paths with -v DJ Delorie
@ 2020-02-19 18:00 ` Carlos O'Donell
  2020-02-19 18:09 ` Joseph Myers
  1 sibling, 0 replies; 6+ messages in thread
From: Carlos O'Donell @ 2020-02-19 18:00 UTC (permalink / raw)
  To: DJ Delorie, libc-alpha

On 2/19/20 12:50 PM, DJ Delorie wrote:
> From 74548bffe27d8c37c78b25164b740dd591f681a9 Mon Sep 17 00:00:00 2001
> From: DJ Delorie <dj@redhat.com>
> Date: Wed, 19 Feb 2020 12:31:38 -0500
> Subject: ldconfig: trace origin paths with -v
> 
> With this patch, -v turns on a "from" trace for each directory
> searched, that tells you WHY that directory is being searched -
> is it a builtin, from the command line, or from some config file?

LGTM. It adds quite a bit more useful information to ldconfig -v for
developers and helps diagnose weird conf files issues! Thanks!

Reviewed-by: Carlos O'Donell <carlos@redhat.com>
 
> diff --git a/elf/ldconfig.c b/elf/ldconfig.c
> index 681ed78496..19ed04cf4d 100644
> --- a/elf/ldconfig.c
> +++ b/elf/ldconfig.c
> @@ -79,6 +79,8 @@ struct dir_entry
>    int flag;
>    ino64_t ino;
>    dev_t dev;
> +  const char *from_file;
> +  int from_line;

OK, file and line.

>    struct dir_entry *next;
>  };
>  
> @@ -344,7 +346,12 @@ add_single_dir (struct dir_entry *entry, int verbose)
>        if (ptr->ino == entry->ino && ptr->dev == entry->dev)
>  	{
>  	  if (opt_verbose && verbose)
> -	    error (0, 0, _("Path `%s' given more than once"), entry->path);
> +	    {
> +	      error (0, 0, _("Path `%s' given more than once"), entry->path);
> +	      fprintf (stderr, "(from %s:%d and %s:%d)\n",
> +		       entry->from_file, entry->from_line,
> +		       ptr->from_file, ptr->from_line);
> +	    }

OK, add the file:lineno for the original and conflict and print to stderr.

>  	  /* Use the newer information.  */
>  	  ptr->flag = entry->flag;
>  	  free (entry->path);
> @@ -363,12 +370,15 @@ add_single_dir (struct dir_entry *entry, int verbose)
>  
>  /* Add one directory to the list of directories to process.  */
>  static void
> -add_dir (const char *line)
> +add_dir_1 (const char *line, const char *from_file, int from_line)
>  {
>    unsigned int i;
>    struct dir_entry *entry = xmalloc (sizeof (struct dir_entry));
>    entry->next = NULL;
>  
> +  entry->from_file = strdup (from_file);
> +  entry->from_line = from_line;

OK.

> +
>    /* Search for an '=' sign.  */
>    entry->path = xstrdup (line);
>    char *equal_sign = strchr (entry->path, '=');
> @@ -428,6 +438,11 @@ add_dir (const char *line)
>      free (path);
>  }
>  
> +static void
> +add_dir (const char *line)
> +{
> +  add_dir_1 (line, "<builtin>", 0);

OK.

> +}
>  
>  static int
>  chroot_stat (const char *real_path, const char *path, struct stat64 *st)
> @@ -672,9 +687,10 @@ search_dir (const struct dir_entry *entry)
>    if (opt_verbose)
>      {
>        if (hwcap != 0)
> -	printf ("%s: (hwcap: %#.16" PRIx64 ")\n", entry->path, hwcap);
> +	printf ("%s: (hwcap: %#.16" PRIx64 ")", entry->path, hwcap);

OK.

>        else
> -	printf ("%s:\n", entry->path);
> +	printf ("%s:", entry->path);
> +      printf (" (from %s:%d)\n", entry->from_file, entry->from_line);

OK, move \n to here and print file:lineno.

>      }
>  
>    char *dir_name;
> @@ -815,6 +831,8 @@ search_dir (const struct dir_entry *entry)
>  	  struct dir_entry *new_entry;
>  
>  	  new_entry = xmalloc (sizeof (struct dir_entry));
> +	  new_entry->from_file = entry->from_file;
> +	  new_entry->from_line = entry->from_line;

OK.

>  	  new_entry->path = xstrdup (file_name);
>  	  new_entry->flag = entry->flag;
>  	  new_entry->next = NULL;
> @@ -1175,7 +1193,7 @@ Warning: ignoring configuration file that cannot be opened: %s"),
>  	    }
>  	}
>        else
> -	add_dir (cp);
> +	add_dir_1 (cp, filename, lineno);

OK.

>      }
>    while (!feof_unlocked (file));
>  
> @@ -1283,7 +1301,7 @@ main (int argc, char **argv)
>  		 _("relative path `%s' used to build cache"),
>  		 argv[i]);
>  	else
> -	  add_dir (argv[i]);
> +	  add_dir_1 (argv[i], "<cmdline>", 0);

OK.

>      }
>  
>    /* The last entry in hwcap_extra is reserved for the "tls" pseudo-hwcap which
> 


-- 
Cheers,
Carlos.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch] ldconfig: trace origin paths with -v
  2020-02-19 17:50 [patch] ldconfig: trace origin paths with -v DJ Delorie
  2020-02-19 18:00 ` Carlos O'Donell
@ 2020-02-19 18:09 ` Joseph Myers
  2020-02-19 20:45   ` DJ Delorie
  1 sibling, 1 reply; 6+ messages in thread
From: Joseph Myers @ 2020-02-19 18:09 UTC (permalink / raw)
  To: DJ Delorie; +Cc: libc-alpha

On Wed, 19 Feb 2020, DJ Delorie wrote:

> @@ -344,7 +346,12 @@ add_single_dir (struct dir_entry *entry, int verbose)
>        if (ptr->ino == entry->ino && ptr->dev == entry->dev)
>  	{
>  	  if (opt_verbose && verbose)
> -	    error (0, 0, _("Path `%s' given more than once"), entry->path);
> +	    {
> +	      error (0, 0, _("Path `%s' given more than once"), entry->path);
> +	      fprintf (stderr, "(from %s:%d and %s:%d)\n",
> +		       entry->from_file, entry->from_line,
> +		       ptr->from_file, ptr->from_line);

This looks like it's a message, for human readers rather than for 
automated parsing, giving extra information about the previous message.  
That previous message is marked up with _() for translation, and as this 
new message includes English words, it should be marked up for translation 
as well.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch] ldconfig: trace origin paths with -v
  2020-02-19 18:09 ` Joseph Myers
@ 2020-02-19 20:45   ` DJ Delorie
  2020-03-12 15:26     ` Carlos O'Donell via Libc-alpha
  0 siblings, 1 reply; 6+ messages in thread
From: DJ Delorie @ 2020-02-19 20:45 UTC (permalink / raw)
  To: Joseph Myers; +Cc: libc-alpha

Joseph Myers <joseph@codesourcery.com> writes:
> This looks like it's a message, for human readers rather than for 

There were two; the other was just "(from %s:%d)".  Marked.  Anything
else?  I note that the "(hwcap ...)" string is NOT marked up but that
may be... well, more correct than not.

From 7a59c4ddc48db342140e65a80bc5a00560d380d2 Mon Sep 17 00:00:00 2001
From: DJ Delorie <dj@redhat.com>
Date: Wed, 19 Feb 2020 12:31:38 -0500
Subject: ldconfig: trace origin paths with -v

With this patch, -v turns on a "from" trace for each directory
searched, that tells you WHY that directory is being searched -
is it a builtin, from the command line, or from some config file?

diff --git a/elf/ldconfig.c b/elf/ldconfig.c
index 681ed78496..71178d416b 100644
--- a/elf/ldconfig.c
+++ b/elf/ldconfig.c
@@ -79,6 +79,8 @@ struct dir_entry
   int flag;
   ino64_t ino;
   dev_t dev;
+  const char *from_file;
+  int from_line;
   struct dir_entry *next;
 };
 
@@ -344,7 +346,12 @@ add_single_dir (struct dir_entry *entry, int verbose)
       if (ptr->ino == entry->ino && ptr->dev == entry->dev)
 	{
 	  if (opt_verbose && verbose)
-	    error (0, 0, _("Path `%s' given more than once"), entry->path);
+	    {
+	      error (0, 0, _("Path `%s' given more than once"), entry->path);
+	      fprintf (stderr, _("(from %s:%d and %s:%d)\n"),
+		       entry->from_file, entry->from_line,
+		       ptr->from_file, ptr->from_line);
+	    }
 	  /* Use the newer information.  */
 	  ptr->flag = entry->flag;
 	  free (entry->path);
@@ -363,12 +370,15 @@ add_single_dir (struct dir_entry *entry, int verbose)
 
 /* Add one directory to the list of directories to process.  */
 static void
-add_dir (const char *line)
+add_dir_1 (const char *line, const char *from_file, int from_line)
 {
   unsigned int i;
   struct dir_entry *entry = xmalloc (sizeof (struct dir_entry));
   entry->next = NULL;
 
+  entry->from_file = strdup (from_file);
+  entry->from_line = from_line;
+
   /* Search for an '=' sign.  */
   entry->path = xstrdup (line);
   char *equal_sign = strchr (entry->path, '=');
@@ -428,6 +438,11 @@ add_dir (const char *line)
     free (path);
 }
 
+static void
+add_dir (const char *line)
+{
+  add_dir_1 (line, "<builtin>", 0);
+}
 
 static int
 chroot_stat (const char *real_path, const char *path, struct stat64 *st)
@@ -672,9 +687,10 @@ search_dir (const struct dir_entry *entry)
   if (opt_verbose)
     {
       if (hwcap != 0)
-	printf ("%s: (hwcap: %#.16" PRIx64 ")\n", entry->path, hwcap);
+	printf ("%s: (hwcap: %#.16" PRIx64 ")", entry->path, hwcap);
       else
-	printf ("%s:\n", entry->path);
+	printf ("%s:", entry->path);
+      printf (_(" (from %s:%d)\n"), entry->from_file, entry->from_line);
     }
 
   char *dir_name;
@@ -815,6 +831,8 @@ search_dir (const struct dir_entry *entry)
 	  struct dir_entry *new_entry;
 
 	  new_entry = xmalloc (sizeof (struct dir_entry));
+	  new_entry->from_file = entry->from_file;
+	  new_entry->from_line = entry->from_line;
 	  new_entry->path = xstrdup (file_name);
 	  new_entry->flag = entry->flag;
 	  new_entry->next = NULL;
@@ -1175,7 +1193,7 @@ Warning: ignoring configuration file that cannot be opened: %s"),
 	    }
 	}
       else
-	add_dir (cp);
+	add_dir_1 (cp, filename, lineno);
     }
   while (!feof_unlocked (file));
 
@@ -1283,7 +1301,7 @@ main (int argc, char **argv)
 		 _("relative path `%s' used to build cache"),
 		 argv[i]);
 	else
-	  add_dir (argv[i]);
+	  add_dir_1 (argv[i], "<cmdline>", 0);
     }
 
   /* The last entry in hwcap_extra is reserved for the "tls" pseudo-hwcap which


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [patch] ldconfig: trace origin paths with -v
  2020-02-19 20:45   ` DJ Delorie
@ 2020-03-12 15:26     ` Carlos O'Donell via Libc-alpha
  2020-03-12 19:08       ` DJ Delorie via Libc-alpha
  0 siblings, 1 reply; 6+ messages in thread
From: Carlos O'Donell via Libc-alpha @ 2020-03-12 15:26 UTC (permalink / raw)
  To: DJ Delorie, Joseph Myers; +Cc: libc-alpha

On 2/19/20 3:45 PM, DJ Delorie wrote:
> Joseph Myers <joseph@codesourcery.com> writes:
>> This looks like it's a message, for human readers rather than for 
> 
> There were two; the other was just "(from %s:%d)".  Marked.  Anything
> else?  I note that the "(hwcap ...)" string is NOT marked up but that
> may be... well, more correct than not.
> 
> From 7a59c4ddc48db342140e65a80bc5a00560d380d2 Mon Sep 17 00:00:00 2001
> From: DJ Delorie <dj@redhat.com>
> Date: Wed, 19 Feb 2020 12:31:38 -0500
> Subject: ldconfig: trace origin paths with -v
> 
> With this patch, -v turns on a "from" trace for each directory
> searched, that tells you WHY that directory is being searched -
> is it a builtin, from the command line, or from some config file?
> 

LTGM. FAOD I do not think <builtin> and <cmdline> need translation.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> diff --git a/elf/ldconfig.c b/elf/ldconfig.c
> index 681ed78496..71178d416b 100644
> --- a/elf/ldconfig.c
> +++ b/elf/ldconfig.c
> @@ -79,6 +79,8 @@ struct dir_entry
>    int flag;
>    ino64_t ino;
>    dev_t dev;
> +  const char *from_file;
> +  int from_line;

OK.

>    struct dir_entry *next;
>  };
>  
> @@ -344,7 +346,12 @@ add_single_dir (struct dir_entry *entry, int verbose)
>        if (ptr->ino == entry->ino && ptr->dev == entry->dev)
>  	{
>  	  if (opt_verbose && verbose)
> -	    error (0, 0, _("Path `%s' given more than once"), entry->path);
> +	    {
> +	      error (0, 0, _("Path `%s' given more than once"), entry->path);
> +	      fprintf (stderr, _("(from %s:%d and %s:%d)\n"),
> +		       entry->from_file, entry->from_line,
> +		       ptr->from_file, ptr->from_line);

OK, use the file and line data for an effective message.

> +	    }
>  	  /* Use the newer information.  */
>  	  ptr->flag = entry->flag;
>  	  free (entry->path);
> @@ -363,12 +370,15 @@ add_single_dir (struct dir_entry *entry, int verbose)
>  
>  /* Add one directory to the list of directories to process.  */
>  static void
> -add_dir (const char *line)
> +add_dir_1 (const char *line, const char *from_file, int from_line)
>  {
>    unsigned int i;
>    struct dir_entry *entry = xmalloc (sizeof (struct dir_entry));
>    entry->next = NULL;
>  
> +  entry->from_file = strdup (from_file);
> +  entry->from_line = from_line;

OK.

> +
>    /* Search for an '=' sign.  */
>    entry->path = xstrdup (line);
>    char *equal_sign = strchr (entry->path, '=');
> @@ -428,6 +438,11 @@ add_dir (const char *line)
>      free (path);
>  }
>  
> +static void
> +add_dir (const char *line)
> +{
> +  add_dir_1 (line, "<builtin>", 0);
> +}

My opinion is that this doesn't need translation since it's a technical
reference to a specific type of entry.

>  
>  static int
>  chroot_stat (const char *real_path, const char *path, struct stat64 *st)
> @@ -672,9 +687,10 @@ search_dir (const struct dir_entry *entry)
>    if (opt_verbose)
>      {
>        if (hwcap != 0)
> -	printf ("%s: (hwcap: %#.16" PRIx64 ")\n", entry->path, hwcap);
> +	printf ("%s: (hwcap: %#.16" PRIx64 ")", entry->path, hwcap);
>        else
> -	printf ("%s:\n", entry->path);
> +	printf ("%s:", entry->path);
> +      printf (_(" (from %s:%d)\n"), entry->from_file, entry->from_line);

OK.

>      }
>  
>    char *dir_name;
> @@ -815,6 +831,8 @@ search_dir (const struct dir_entry *entry)
>  	  struct dir_entry *new_entry;
>  
>  	  new_entry = xmalloc (sizeof (struct dir_entry));
> +	  new_entry->from_file = entry->from_file;
> +	  new_entry->from_line = entry->from_line;

OK.

>  	  new_entry->path = xstrdup (file_name);
>  	  new_entry->flag = entry->flag;
>  	  new_entry->next = NULL;
> @@ -1175,7 +1193,7 @@ Warning: ignoring configuration file that cannot be opened: %s"),
>  	    }
>  	}
>        else
> -	add_dir (cp);
> +	add_dir_1 (cp, filename, lineno);

OK.

>      }
>    while (!feof_unlocked (file));
>  
> @@ -1283,7 +1301,7 @@ main (int argc, char **argv)
>  		 _("relative path `%s' used to build cache"),
>  		 argv[i]);
>  	else
> -	  add_dir (argv[i]);
> +	  add_dir_1 (argv[i], "<cmdline>", 0);

My opinion is that this doesn't need translation since it's a technical
reference to a specific type of entry.

>      }
>  
>    /* The last entry in hwcap_extra is reserved for the "tls" pseudo-hwcap which
> 


-- 
Cheers,
Carlos.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch] ldconfig: trace origin paths with -v
  2020-03-12 15:26     ` Carlos O'Donell via Libc-alpha
@ 2020-03-12 19:08       ` DJ Delorie via Libc-alpha
  0 siblings, 0 replies; 6+ messages in thread
From: DJ Delorie via Libc-alpha @ 2020-03-12 19:08 UTC (permalink / raw)
  To: Carlos O'Donell; +Cc: libc-alpha, joseph

"Carlos O'Donell" <carlos@redhat.com> writes:
> LTGM. FAOD I do not think <builtin> and <cmdline> need translation.

Thanks, pushed.

> Reviewed-by: Carlos O'Donell <carlos@redhat.com>

and once again FORGOT THIS PART :-P


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-03-12 19:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-19 17:50 [patch] ldconfig: trace origin paths with -v DJ Delorie
2020-02-19 18:00 ` Carlos O'Donell
2020-02-19 18:09 ` Joseph Myers
2020-02-19 20:45   ` DJ Delorie
2020-03-12 15:26     ` Carlos O'Donell via Libc-alpha
2020-03-12 19:08       ` DJ Delorie via Libc-alpha

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).