Re: [RFC 1/2] Remove linux-waitpid.c debugging code

[Simon Marchi <simark at simark dot ca>]

On 2019-05-30 5:30 p.m., Tom Tromey wrote:
> The debugging code in linux-waitpid.c is one of the few remaining
> spots that depends on the gdb/gdbserver difference.
> 
> My first thought was that this code is not extremely useful, so this
> patch removes this code.  (However, if it is actually useful to
> someone, we could make it work by introducing a new abstraction.)
> 
> gdb/ChangeLog
> 2019-05-30  Tom Tromey  <tom@tromey.com>
> 
> 	* nat/linux-waitpid.c: Don't include server.h.
> 	(linux_debug): Remove.
> 	(my_waitpid): Update.
> ---
>  gdb/ChangeLog           |  6 ++++++
>  gdb/nat/linux-waitpid.c | 34 +---------------------------------
>  2 files changed, 7 insertions(+), 33 deletions(-)
> 
> diff --git a/gdb/nat/linux-waitpid.c b/gdb/nat/linux-waitpid.c
> index a7d11ab8d32..298032dff9a 100644
> --- a/gdb/nat/linux-waitpid.c
> +++ b/gdb/nat/linux-waitpid.c
> @@ -19,35 +19,10 @@
>  
>  #include "common/common-defs.h"
>  
> -#ifdef GDBSERVER
> -/* FIXME: server.h is required for the definition of debug_threads
> -   which is used in the gdbserver-specific debug printing in
> -   linux_debug.  This code should be made available to GDB also,
> -   but the lack of a suitable flag to enable it prevents this.  */
> -#include "server.h"
> -#endif
> -
>  #include "linux-nat.h"
>  #include "linux-waitpid.h"
>  #include "common/gdb_wait.h"
>  
> -/* Print debugging output based on the format string FORMAT and
> -   its parameters.  */
> -
> -static inline void ATTRIBUTE_PRINTF (1,2)
> -linux_debug (const char *format, ...)
> -{
> -#ifdef GDBSERVER
> -  if (debug_threads)
> -    {
> -      va_list args;
> -      va_start (args, format);
> -      debug_vprintf (format, args);
> -      va_end (args);
> -    }
> -#endif
> -}
> -
>  /* Convert wait status STATUS to a string.  Used for printing debug
>     messages only.  */
>  
> @@ -79,20 +54,13 @@ status_to_str (int status)
>  int
>  my_waitpid (int pid, int *status, int flags)
>  {
> -  int ret, out_errno;
> -
> -  linux_debug ("my_waitpid (%d, 0x%x)\n", pid, flags);
> +  int ret;
>  
>    do
>      {
>        ret = waitpid (pid, status, flags);
>      }
>    while (ret == -1 && errno == EINTR);
> -  out_errno = errno;
> -
> -  linux_debug ("my_waitpid (%d, 0x%x): status(%x), %d\n",
> -	       pid, flags, (ret > 0 && status != NULL) ? *status : -1, ret);
>  
> -  errno = out_errno;
>    return ret;
>  }
> 

Pedro probably has a stronger opinion about this (as he is probably the one who
has spent the most time staring at those logs), but I would think that this
logging is useful when debugging interactions with the kernel.

Simon