diff mbox

[1/5] libthread_db: debug output should go to gdb_stdlog

Message ID 1418748834-27545-2-git-send-email-palves@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves Dec. 16, 2014, 4:53 p.m. UTC 
  Some debug output in linux-thread-db.c was being sent to gdb_stdout,
and some to gdb_stderr, while the right place to send debug output to is
gdb_stdlog.

gdb/
2014-12-16  Pedro Alves  <palves@redhat.com>

	* linux-thread-db.c (thread_db_find_new_threads_silently)
	(try_thread_db_load_1, try_thread_db_load, thread_db_load_search)
	(find_new_threads_once): Print debug output on gdb_stdlog.
---
 gdb/linux-thread-db.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

Comments

Yao Qi Dec. 17, 2014, 8:02 a.m. UTC | #1 
Pedro Alves <palves@redhat.com> writes:

>    if (libthread_db_debug || *libthread_db_search_path)
>      {
> +      struct ui_file *file;
>        const char *library;
>  
>        library = dladdr_to_soname (*info->td_ta_new_p);
>        if (library == NULL)
>  	library = LIBTHREAD_DB_SO;
>  
> -      printf_unfiltered (_("Using host libthread_db library \"%s\".\n"),
> -			 library);
> +      file = *libthread_db_search_path != '\0' ? gdb_stdout : gdb_stdlog;

Nit: why don't we check libthread_db_debug instead? like:

      file = libthread_db_debug ? gdb_stdlog : gdb_stdout;

> +      fprintf_unfiltered (file, _("Using host libthread_db library \"%s\".\n"),
> +			  library);
Pedro Alves Dec. 17, 2014, 1:45 p.m. UTC | #2 
On 12/17/2014 08:02 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>>    if (libthread_db_debug || *libthread_db_search_path)
>>      {
>> +      struct ui_file *file;
>>        const char *library;
>>  
>>        library = dladdr_to_soname (*info->td_ta_new_p);
>>        if (library == NULL)
>>  	library = LIBTHREAD_DB_SO;
>>  
>> -      printf_unfiltered (_("Using host libthread_db library \"%s\".\n"),
>> -			 library);
>> +      file = *libthread_db_search_path != '\0' ? gdb_stdout : gdb_stdlog;
> 
> Nit: why don't we check libthread_db_debug instead? like:
> 
>       file = libthread_db_debug ? gdb_stdlog : gdb_stdout;

Let me answer that by adding a comment.  Does this make it clearer?

  if (*libthread_db_search_path || libthread_db_debug)
    {
      struct ui_file *file;
      const char *library;

      library = dladdr_to_soname (*info->td_ta_new_p);
      if (library == NULL)
	library = LIBTHREAD_DB_SO;

      /* If we'd print this to gdb_stdout when debug output is
	 disabled, still print it to gdb_stdout if debug output is
	 enabled.  User visible output should not depend on debug
	 settings.  */
      file = *libthread_db_search_path != '\0' ? gdb_stdout : gdb_stdlog;
      fprintf_unfiltered (file, _("Using host libthread_db library \"%s\".\n"),
			  library);
    }

Thanks,
Pedro Alves
Yao Qi Dec. 17, 2014, 2:08 p.m. UTC | #3 
Pedro Alves <palves@redhat.com> writes:

> Let me answer that by adding a comment.  Does this make it clearer?

Yes, it is clearer....

>
>   if (*libthread_db_search_path || libthread_db_debug)

... and it is also important to exchange them in the condition.

>     {
>       struct ui_file *file;
>       const char *library;
>
>       library = dladdr_to_soname (*info->td_ta_new_p);
>       if (library == NULL)
> 	library = LIBTHREAD_DB_SO;
>
>       /* If we'd print this to gdb_stdout when debug output is
> 	 disabled, still print it to gdb_stdout if debug output is
> 	 enabled.  User visible output should not depend on debug
> 	 settings.  */
diff mbox

Patch

diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index c49b567..a405603 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -671,7 +671,7 @@  thread_db_find_new_threads_silently (ptid_t ptid)
   if (except.reason < 0)
     {
       if (libthread_db_debug)
-	exception_fprintf (gdb_stderr, except,
+	exception_fprintf (gdb_stdlog, except,
 			   "Warning: thread_db_find_new_threads_silently: ");
 
       /* There is a bug fixed between nptl 2.6.1 and 2.7 by
@@ -753,8 +753,8 @@  try_thread_db_load_1 (struct thread_db_info *info)
   if (err != TD_OK)
     {
       if (libthread_db_debug)
-	printf_unfiltered (_("td_ta_new failed: %s\n"),
-			   thread_db_err_str (err));
+	fprintf_unfiltered (gdb_stdlog, _("td_ta_new failed: %s\n"),
+			    thread_db_err_str (err));
       else
         switch (err)
           {
@@ -814,14 +814,16 @@  try_thread_db_load_1 (struct thread_db_info *info)
 
   if (libthread_db_debug || *libthread_db_search_path)
     {
+      struct ui_file *file;
       const char *library;
 
       library = dladdr_to_soname (*info->td_ta_new_p);
       if (library == NULL)
 	library = LIBTHREAD_DB_SO;
 
-      printf_unfiltered (_("Using host libthread_db library \"%s\".\n"),
-			 library);
+      file = *libthread_db_search_path != '\0' ? gdb_stdout : gdb_stdlog;
+      fprintf_unfiltered (file, _("Using host libthread_db library \"%s\".\n"),
+			  library);
     }
 
   /* The thread library was detected.  Activate the thread_db target
@@ -846,8 +848,9 @@  try_thread_db_load (const char *library, int check_auto_load_safe)
   struct thread_db_info *info;
 
   if (libthread_db_debug)
-    printf_unfiltered (_("Trying host libthread_db library: %s.\n"),
-                       library);
+    fprintf_unfiltered (gdb_stdlog,
+			_("Trying host libthread_db library: %s.\n"),
+			library);
 
   if (check_auto_load_safe)
     {
@@ -856,7 +859,8 @@  try_thread_db_load (const char *library, int check_auto_load_safe)
 	  /* Do not print warnings by file_is_auto_load_safe if the library does
 	     not exist at this place.  */
 	  if (libthread_db_debug)
-	    printf_unfiltered (_("open failed: %s.\n"), safe_strerror (errno));
+	    fprintf_unfiltered (gdb_stdlog, _("open failed: %s.\n"),
+				safe_strerror (errno));
 	  return 0;
 	}
 
@@ -871,7 +875,7 @@  try_thread_db_load (const char *library, int check_auto_load_safe)
   if (handle == NULL)
     {
       if (libthread_db_debug)
-	printf_unfiltered (_("dlopen failed: %s.\n"), dlerror ());
+	fprintf_unfiltered (gdb_stdlog, _("dlopen failed: %s.\n"), dlerror ());
       return 0;
     }
 
@@ -885,7 +889,7 @@  try_thread_db_load (const char *library, int check_auto_load_safe)
           const char *const libpath = dladdr_to_soname (td_init);
 
           if (libpath != NULL)
-            printf_unfiltered (_("Host %s resolved to: %s.\n"),
+            fprintf_unfiltered (gdb_stdlog, _("Host %s resolved to: %s.\n"),
                                library, libpath);
         }
     }
@@ -1076,7 +1080,8 @@  thread_db_load_search (void)
 
   do_cleanups (cleanups);
   if (libthread_db_debug)
-    printf_unfiltered (_("thread_db_load_search returning %d\n"), rc);
+    fprintf_unfiltered (gdb_stdlog,
+			_("thread_db_load_search returning %d\n"), rc);
   return rc;
 }
 
@@ -1683,11 +1688,12 @@  find_new_threads_once (struct thread_db_info *info, int iteration,
   if (libthread_db_debug)
     {
       if (except.reason < 0)
-	exception_fprintf (gdb_stderr, except,
+	exception_fprintf (gdb_stdlog, except,
 			   "Warning: find_new_threads_once: ");
 
-      printf_filtered (_("Found %d new threads in iteration %d.\n"),
-		       data.new_threads, iteration);
+      fprintf_unfiltered (gdb_stdlog,
+			  _("Found %d new threads in iteration %d.\n"),
+			  data.new_threads, iteration);
     }
 
   if (errp != NULL)