Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] Remove set debug lin-lwp-async, new ui_file.to_write_async_safe
@ 2011-05-10 20:47 Doug Evans
  2011-05-10 20:51 ` Doug Evans
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Doug Evans @ 2011-05-10 20:47 UTC (permalink / raw)
  To: gdb-patches, pedro

Hi.

This patch does two things.

1) Remove debug_linux_nat_async, replacing all uses with debug_linux_nat.

2) Fix the debug printf in sigchld_handler.

Adding to_write_async_safe (and the associated scaffolding) just
for sigchld_handler may seem like a lot, but I *like* having a
debug printf in sigchld_handler.  And given that it might be
invoked more often now, I think we need to fix it and use a
more signal-safe debug printer.
I haven't added the higher level of abstraction (fprintf_unfiltered_async_safe
or some such) and, e.g. the associated gettimeofday handling.
That can be done later if it's useful enough.

Pedro mentioned there being a lot more output because of this.
I like what's there now, but I can imagine some tweaking may
be needed.  I'd like to add some more debug printfs here,
and I wouldn't mind having "set debug lin-lwp N" where N would
specify a level of verbosity (0,1,2 at most probably).

Ok to check in?

2011-05-10  Doug Evans  <dje@google.com>

	* linux-nat.c (debug_linux_nat_async): Delete.
	Replace all references to use debug_linux_nat instead.
	(show_debug_linux_nat_async): Delete.
	(sigchld_handler): Call ui_file_write_async_safe instead of
	fprintf_unfiltered.
	(_initialize_linux_nat): Remove `set debug lin-lwp-async'.
	* ui-file.c (struct ui_file): New member to_write_async_safe.
	(null_file_write_async_safe): New function.
	(ui_file_write_async_safe): New function.
	(set_ui_file_write_async_safe): New function.
	(stdio_file_new): Initialize to_write_async_safe.
	(stdio_file_write_async_safe): New function.
	* ui-file.h (ui_file_write_async_safe_ftype): New typedef.
	(set_ui_file_write_async_safe): Declare.
	(ui_file_write_async_safe): Declare.

	doc/
	* gdb.texinfo (Completion): Update example.

Index: linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-nat.c,v
retrieving revision 1.203
diff -u -p -r1.203 linux-nat.c
--- linux-nat.c	9 May 2011 18:43:56 -0000	1.203
+++ linux-nat.c	10 May 2011 20:29:30 -0000
@@ -198,16 +198,6 @@ show_debug_linux_nat (struct ui_file *fi
 		    value);
 }
 
-static int debug_linux_nat_async = 0;
-static void
-show_debug_linux_nat_async (struct ui_file *file, int from_tty,
-			    struct cmd_list_element *c, const char *value)
-{
-  fprintf_filtered (file,
-		    _("Debugging of GNU/Linux async lwp module is %s.\n"),
-		    value);
-}
-
 static int disable_randomization = 1;
 
 static void
@@ -3260,7 +3250,7 @@ linux_nat_wait_1 (struct target_ops *ops
   int status = 0;
   pid_t pid;
 
-  if (debug_linux_nat_async)
+  if (debug_linux_nat)
     fprintf_unfiltered (gdb_stdlog, "LLW: enter\n");
 
   /* The first time we get here after starting a new inferior, we may
@@ -3303,7 +3293,7 @@ retry:
     {
       ourstatus->kind = TARGET_WAITKIND_IGNORE;
 
-      if (debug_linux_nat_async)
+      if (debug_linux_nat)
 	fprintf_unfiltered (gdb_stdlog, "LLW: exit (no resumed LWP)\n");
 
       restore_child_signals_mask (&prev_mask);
@@ -3546,7 +3536,7 @@ retry:
 		  /* No interesting event.  */
 		  ourstatus->kind = TARGET_WAITKIND_IGNORE;
 
-		  if (debug_linux_nat_async)
+		  if (debug_linux_nat)
 		    fprintf_unfiltered (gdb_stdlog, "LLW: exit (ignore)\n");
 
 		  restore_child_signals_mask (&prev_mask);
@@ -3561,7 +3551,7 @@ retry:
 	  /* No interesting event for PID yet.  */
 	  ourstatus->kind = TARGET_WAITKIND_IGNORE;
 
-	  if (debug_linux_nat_async)
+	  if (debug_linux_nat)
 	    fprintf_unfiltered (gdb_stdlog, "LLW: exit (ignore)\n");
 
 	  restore_child_signals_mask (&prev_mask);
@@ -3690,7 +3680,7 @@ retry:
   else
     store_waitstatus (ourstatus, status);
 
-  if (debug_linux_nat_async)
+  if (debug_linux_nat)
     fprintf_unfiltered (gdb_stdlog, "LLW: exit\n");
 
   restore_child_signals_mask (&prev_mask);
@@ -5416,8 +5406,9 @@ sigchld_handler (int signo)
 {
   int old_errno = errno;
 
-  if (debug_linux_nat_async)
-    fprintf_unfiltered (gdb_stdlog, "sigchld\n");
+  if (debug_linux_nat)
+    ui_file_write_async_safe (gdb_stdlog,
+			      "sigchld\n", sizeof ("sigchld\n") - 1);
 
   if (signo == SIGCHLD
       && linux_nat_event_pipe[0] != -1)
@@ -5801,15 +5792,6 @@ Enables printf debugging output."),
 			    show_debug_linux_nat,
 			    &setdebuglist, &showdebuglist);
 
-  add_setshow_zinteger_cmd ("lin-lwp-async", class_maintenance,
-			    &debug_linux_nat_async, _("\
-Set debugging of GNU/Linux async lwp module."), _("\
-Show debugging of GNU/Linux async lwp module."), _("\
-Enables printf debugging output."),
-			    NULL,
-			    show_debug_linux_nat_async,
-			    &setdebuglist, &showdebuglist);
-
   /* Save this mask as the default.  */
   sigprocmask (SIG_SETMASK, NULL, &normal_mask);
 
Index: ui-file.c
===================================================================
RCS file: /cvs/src/src/gdb/ui-file.c,v
retrieving revision 1.26
diff -u -p -r1.26 ui-file.c
--- ui-file.c	12 Jan 2011 01:23:28 -0000	1.26
+++ ui-file.c	10 May 2011 20:29:30 -0000
@@ -30,6 +30,7 @@
 
 static ui_file_isatty_ftype null_file_isatty;
 static ui_file_write_ftype null_file_write;
+static ui_file_write_ftype null_file_write_async_safe;
 static ui_file_fputs_ftype null_file_fputs;
 static ui_file_read_ftype null_file_read;
 static ui_file_flush_ftype null_file_flush;
@@ -42,6 +43,7 @@ struct ui_file
     int *magic;
     ui_file_flush_ftype *to_flush;
     ui_file_write_ftype *to_write;
+    ui_file_write_async_safe_ftype *to_write_async_safe;
     ui_file_fputs_ftype *to_fputs;
     ui_file_read_ftype *to_read;
     ui_file_delete_ftype *to_delete;
@@ -61,6 +63,7 @@ ui_file_new (void)
   set_ui_file_data (file, NULL, null_file_delete);
   set_ui_file_flush (file, null_file_flush);
   set_ui_file_write (file, null_file_write);
+  set_ui_file_write_async_safe (file, null_file_write_async_safe);
   set_ui_file_fputs (file, null_file_fputs);
   set_ui_file_read (file, null_file_read);
   set_ui_file_isatty (file, null_file_isatty);
@@ -155,6 +158,14 @@ null_file_fputs (const char *buf, struct
 }
 
 static void
+null_file_write_async_safe (struct ui_file *file,
+			    const char *buf,
+			    long sizeof_buf)
+{
+  return;
+}
+
+static void
 null_file_delete (struct ui_file *file)
 {
   return;
@@ -203,6 +214,14 @@ ui_file_write (struct ui_file *file,
   file->to_write (file, buf, length_buf);
 }
 
+void
+ui_file_write_async_safe (struct ui_file *file,
+			  const char *buf,
+			  long length_buf)
+{
+  file->to_write_async_safe (file, buf, length_buf);
+}
+
 long
 ui_file_read (struct ui_file *file, char *buf, long length_buf)
 {
@@ -247,6 +266,13 @@ set_ui_file_write (struct ui_file *file,
 }
 
 void
+set_ui_file_write_async_safe (struct ui_file *file,
+			      ui_file_write_async_safe_ftype *write_async_safe)
+{
+  file->to_write_async_safe = write_async_safe;
+}
+
+void
 set_ui_file_read (struct ui_file *file, ui_file_read_ftype *read)
 {
   file->to_read = read;
@@ -437,6 +463,7 @@ mem_file_write (struct ui_file *file,
    <stdio.h>'s FILE.  */
 
 static ui_file_write_ftype stdio_file_write;
+static ui_file_write_async_safe_ftype stdio_file_write_async_safe;
 static ui_file_fputs_ftype stdio_file_fputs;
 static ui_file_read_ftype stdio_file_read;
 static ui_file_isatty_ftype stdio_file_isatty;
@@ -465,6 +492,7 @@ stdio_file_new (FILE *file, int close_p)
   set_ui_file_data (ui_file, stdio, stdio_file_delete);
   set_ui_file_flush (ui_file, stdio_file_flush);
   set_ui_file_write (ui_file, stdio_file_write);
+  set_ui_file_write_async_safe (ui_file, stdio_file_write_async_safe);
   set_ui_file_fputs (ui_file, stdio_file_fputs);
   set_ui_file_read (ui_file, stdio_file_read);
   set_ui_file_isatty (ui_file, stdio_file_isatty);
@@ -536,6 +564,22 @@ stdio_file_write (struct ui_file *file, 
 }
 
 static void
+stdio_file_write_async_safe (struct ui_file *file,
+			     const char *buf, long length_buf)
+{
+  struct stdio_file *stdio = ui_file_data (file);
+
+  if (stdio->magic != &stdio_file_magic)
+    {
+      const char *msg = _("stdio_file_write_async_safe: bad magic number\n");
+      write (2, msg, strlen (msg));
+      return;
+    }
+
+  write (fileno (stdio->file), buf, length_buf);
+}
+
+static void
 stdio_file_fputs (const char *linebuffer, struct ui_file *file)
 {
   struct stdio_file *stdio = ui_file_data (file);
Index: ui-file.h
===================================================================
RCS file: /cvs/src/src/gdb/ui-file.h,v
retrieving revision 1.16
diff -u -p -r1.16 ui-file.h
--- ui-file.h	27 Feb 2011 16:25:37 -0000	1.16
+++ ui-file.h	10 May 2011 20:29:30 -0000
@@ -45,6 +45,17 @@ typedef void (ui_file_fputs_ftype) (cons
 extern void set_ui_file_fputs (struct ui_file *stream,
 			       ui_file_fputs_ftype *fputs);
 
+/* This version of "write" is safe for use in signal handlers.
+   It's not guaranteed that all existing output will have been
+   flushed first.
+   Implementations are also free to ignore some or all of the request.
+   fputs_async is not provided as the async versions are rarely used,
+   no point in having both for a rarely used interface.  */
+typedef void (ui_file_write_async_safe_ftype)
+  (struct ui_file *stream, const char *buf, long length_buf);
+extern void set_ui_file_write_async_safe
+  (struct ui_file *stream, ui_file_write_async_safe_ftype *write_async_safe);
+
 typedef long (ui_file_read_ftype) (struct ui_file *stream,
 				   char *buf, long length_buf);
 extern void set_ui_file_read (struct ui_file *stream,
@@ -83,6 +94,9 @@ extern int ui_file_isatty (struct ui_fil
 extern void ui_file_write (struct ui_file *file, const char *buf,
 			   long length_buf);
 
+extern void ui_file_write_async_safe (struct ui_file *file, const char *buf,
+				      long length_buf);
+
 /* NOTE: copies left to right.  */
 extern void ui_file_put (struct ui_file *src,
 			 ui_file_put_method_ftype *write, void *dest);
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.832
diff -u -p -r1.832 gdb.texinfo
--- doc/gdb.texinfo	10 May 2011 16:53:22 -0000	1.832
+++ doc/gdb.texinfo	10 May 2011 20:29:31 -0000
@@ -1592,8 +1592,10 @@ left-hand-side:
 
 @smallexample
 (@value{GDBP}) p gdb_stdout.@kbd{M-?}
-magic      to_delete  to_fputs   to_put     to_rewind  
-to_data    to_flush   to_isatty  to_read    to_write   
+magic                to_fputs             to_rewind
+to_data              to_isatty            to_write
+to_delete            to_put               to_write_async_safe
+to_flush             to_read
 @end smallexample
 
 @noindent
@@ -1607,6 +1609,7 @@ struct ui_file
    int *magic;
    ui_file_flush_ftype *to_flush;
    ui_file_write_ftype *to_write;
+   ui_file_write_async_safe_ftype *to_write_async_safe;
    ui_file_fputs_ftype *to_fputs;
    ui_file_read_ftype *to_read;
    ui_file_delete_ftype *to_delete;


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

* Re: [RFA] Remove set debug lin-lwp-async, new ui_file.to_write_async_safe
  2011-05-10 20:47 [RFA] Remove set debug lin-lwp-async, new ui_file.to_write_async_safe Doug Evans
@ 2011-05-10 20:51 ` Doug Evans
  2011-05-10 21:30 ` Pedro Alves
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Doug Evans @ 2011-05-10 20:51 UTC (permalink / raw)
  To: gdb-patches, pedro

[sorry for the resend]

Oh, btw, Heh, forgot ... testsuite run is in progress.
Will amend as necessary.

On Tue, May 10, 2011 at 1:46 PM, Doug Evans <dje@google.com> wrote:
>
> Hi.
>
> This patch does two things.
>
> 1) Remove debug_linux_nat_async, replacing all uses with debug_linux_nat.
>
> 2) Fix the debug printf in sigchld_handler.
>
> Adding to_write_async_safe (and the associated scaffolding) just
> for sigchld_handler may seem like a lot, but I *like* having a
> debug printf in sigchld_handler.  And given that it might be
> invoked more often now, I think we need to fix it and use a
> more signal-safe debug printer.
> I haven't added the higher level of abstraction (fprintf_unfiltered_async_safe
> or some such) and, e.g. the associated gettimeofday handling.
> That can be done later if it's useful enough.
>
> Pedro mentioned there being a lot more output because of this.
> I like what's there now, but I can imagine some tweaking may
> be needed.  I'd like to add some more debug printfs here,
> and I wouldn't mind having "set debug lin-lwp N" where N would
> specify a level of verbosity (0,1,2 at most probably).
>
> Ok to check in?
>
> 2011-05-10  Doug Evans  <dje@google.com>
>
>        * linux-nat.c (debug_linux_nat_async): Delete.
>        Replace all references to use debug_linux_nat instead.
>        (show_debug_linux_nat_async): Delete.
>        (sigchld_handler): Call ui_file_write_async_safe instead of
>        fprintf_unfiltered.
>        (_initialize_linux_nat): Remove `set debug lin-lwp-async'.
>        * ui-file.c (struct ui_file): New member to_write_async_safe.
>        (null_file_write_async_safe): New function.
>        (ui_file_write_async_safe): New function.
>        (set_ui_file_write_async_safe): New function.
>        (stdio_file_new): Initialize to_write_async_safe.
>        (stdio_file_write_async_safe): New function.
>        * ui-file.h (ui_file_write_async_safe_ftype): New typedef.
>        (set_ui_file_write_async_safe): Declare.
>        (ui_file_write_async_safe): Declare.
>
>        doc/
>        * gdb.texinfo (Completion): Update example.


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

* Re: [RFA] Remove set debug lin-lwp-async, new ui_file.to_write_async_safe
  2011-05-10 20:47 [RFA] Remove set debug lin-lwp-async, new ui_file.to_write_async_safe Doug Evans
  2011-05-10 20:51 ` Doug Evans
@ 2011-05-10 21:30 ` Pedro Alves
  2011-05-10 22:19   ` Doug Evans
  2011-05-10 21:38 ` Pedro Alves
  2011-05-11  2:50 ` Eli Zaretskii
  3 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2011-05-10 21:30 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

On Tuesday 10 May 2011 21:46:41, Doug Evans wrote:
>  static void
> +stdio_file_write_async_safe (struct ui_file *file,
> +                            const char *buf, long length_buf)
> +{
> +  struct stdio_file *stdio = ui_file_data (file);
> +
> +  if (stdio->magic != &stdio_file_magic)
> +    {
> +      const char *msg = _("stdio_file_write_async_safe: bad magic number\n");
> +      write (2, msg, strlen (msg));
> +      return;
> +    }
> +
> +  write (fileno (stdio->file), buf, length_buf);

fileno is not required to be async signal safe by posix, AFAIK.
(nor are strlen or _/gettext either).

[gdbserver calls write/sizeof directly in linux-low.c,
 if you haven't seen it]

-- 
Pedro Alves


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

* Re: [RFA] Remove set debug lin-lwp-async, new ui_file.to_write_async_safe
  2011-05-10 20:47 [RFA] Remove set debug lin-lwp-async, new ui_file.to_write_async_safe Doug Evans
  2011-05-10 20:51 ` Doug Evans
  2011-05-10 21:30 ` Pedro Alves
@ 2011-05-10 21:38 ` Pedro Alves
  2011-05-10 22:20   ` Doug Evans
  2011-05-11  2:50 ` Eli Zaretskii
  3 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2011-05-10 21:38 UTC (permalink / raw)
  To: gdb-patches; +Cc: Doug Evans

On Tuesday 10 May 2011 21:46:41, Doug Evans wrote:
> -  add_setshow_zinteger_cmd ("lin-lwp-async", class_maintenance,
> -                           &debug_linux_nat_async, _("\
> -Set debugging of GNU/Linux async lwp module."), _("\
> -Show debugging of GNU/Linux async lwp module."), _("\
> -Enables printf debugging output."),
> -                           NULL,
> -                           show_debug_linux_nat_async,
> -                           &setdebuglist, &showdebuglist);

BTW, don't forget to remove the documentation of the command
in the user manual as well.

-- 
Pedro Alves


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

* Re: [RFA] Remove set debug lin-lwp-async, new ui_file.to_write_async_safe
  2011-05-10 21:30 ` Pedro Alves
@ 2011-05-10 22:19   ` Doug Evans
  2011-05-11  7:39     ` Jan Kratochvil
  0 siblings, 1 reply; 12+ messages in thread
From: Doug Evans @ 2011-05-10 22:19 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On Tue, May 10, 2011 at 2:30 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> On Tuesday 10 May 2011 21:46:41, Doug Evans wrote:
>>  static void
>> +stdio_file_write_async_safe (struct ui_file *file,
>> +                            const char *buf, long length_buf)
>> +{
>> +  struct stdio_file *stdio = ui_file_data (file);
>> +
>> +  if (stdio->magic != &stdio_file_magic)
>> +    {
>> +      const char *msg = _("stdio_file_write_async_safe: bad magic number\n");
>> +      write (2, msg, strlen (msg));
>> +      return;
>> +    }
>> +
>> +  write (fileno (stdio->file), buf, length_buf);
>
> fileno is not required to be async signal safe by posix, AFAIK.
> (nor are strlen or _/gettext either).

If it became an issue, one could record fileno ahead of time.
For gettext(), I'd be happy with just removing it for this particular case.

> [gdbserver calls write/sizeof directly in linux-low.c,
>  if you haven't seen it]

Yeah.


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

* Re: [RFA] Remove set debug lin-lwp-async, new ui_file.to_write_async_safe
  2011-05-10 21:38 ` Pedro Alves
@ 2011-05-10 22:20   ` Doug Evans
  0 siblings, 0 replies; 12+ messages in thread
From: Doug Evans @ 2011-05-10 22:20 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On Tue, May 10, 2011 at 2:38 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> On Tuesday 10 May 2011 21:46:41, Doug Evans wrote:
>> -  add_setshow_zinteger_cmd ("lin-lwp-async", class_maintenance,
>> -                           &debug_linux_nat_async, _("\
>> -Set debugging of GNU/Linux async lwp module."), _("\
>> -Show debugging of GNU/Linux async lwp module."), _("\
>> -Enables printf debugging output."),
>> -                           NULL,
>> -                           show_debug_linux_nat_async,
>> -                           &setdebuglist, &showdebuglist);
>
> BTW, don't forget to remove the documentation of the command
> in the user manual as well.

Ah, righto.


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

* Re: [RFA] Remove set debug lin-lwp-async, new ui_file.to_write_async_safe
  2011-05-10 20:47 [RFA] Remove set debug lin-lwp-async, new ui_file.to_write_async_safe Doug Evans
                   ` (2 preceding siblings ...)
  2011-05-10 21:38 ` Pedro Alves
@ 2011-05-11  2:50 ` Eli Zaretskii
  3 siblings, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2011-05-11  2:50 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches, pedro

> Date: Tue, 10 May 2011 13:46:41 -0700 (PDT)
> From: dje@google.com (Doug Evans)
> 
> 	doc/
> 	* gdb.texinfo (Completion): Update example.

This part is okay.

Thanks.


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

* Re: [RFA] Remove set debug lin-lwp-async, new ui_file.to_write_async_safe
  2011-05-10 22:19   ` Doug Evans
@ 2011-05-11  7:39     ` Jan Kratochvil
  2011-05-11 15:50       ` Doug Evans
  0 siblings, 1 reply; 12+ messages in thread
From: Jan Kratochvil @ 2011-05-11  7:39 UTC (permalink / raw)
  To: Doug Evans; +Cc: Pedro Alves, gdb-patches

On Wed, 11 May 2011 00:19:36 +0200, Doug Evans wrote:
> If it became an issue, one could record fileno ahead of time.

Isn't it simple enough in stdio_file_new?


> For gettext(), I'd be happy with just removing it for this particular case.

One can call it from _initialize_ui_file.  There are no catalogues now but
they should be.


Thanks,
Jan


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

* Re: [RFA] Remove set debug lin-lwp-async, new ui_file.to_write_async_safe
  2011-05-11  7:39     ` Jan Kratochvil
@ 2011-05-11 15:50       ` Doug Evans
  2011-05-13 17:32         ` Doug Evans
  0 siblings, 1 reply; 12+ messages in thread
From: Doug Evans @ 2011-05-11 15:50 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Pedro Alves, gdb-patches

On Wed, May 11, 2011 at 12:39 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Wed, 11 May 2011 00:19:36 +0200, Doug Evans wrote:
>> If it became an issue, one could record fileno ahead of time.
>
> Isn't it simple enough in stdio_file_new?

Sounds like what I would do.

>> For gettext(), I'd be happy with just removing it for this particular case.
>
> One can call it from _initialize_ui_file.  There are no catalogues now but
> they should be.

If it's important enough, sure.


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

* Re: [RFA] Remove set debug lin-lwp-async, new ui_file.to_write_async_safe
  2011-05-11 15:50       ` Doug Evans
@ 2011-05-13 17:32         ` Doug Evans
  2011-05-13 19:05           ` Stan Shebs
  0 siblings, 1 reply; 12+ messages in thread
From: Doug Evans @ 2011-05-13 17:32 UTC (permalink / raw)
  To: Jan Kratochvil, Pedro Alves; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1845 bytes --]

On Wed, May 11, 2011 at 8:49 AM, Doug Evans <dje@google.com> wrote:
> On Wed, May 11, 2011 at 12:39 AM, Jan Kratochvil
> <jan.kratochvil@redhat.com> wrote:
>> On Wed, 11 May 2011 00:19:36 +0200, Doug Evans wrote:
>>> If it became an issue, one could record fileno ahead of time.
>>
>> Isn't it simple enough in stdio_file_new?
>
> Sounds like what I would do.
>
>>> For gettext(), I'd be happy with just removing it for this particular case.
>>
>> One can call it from _initialize_ui_file.  There are no catalogues now but
>> they should be.
>
> If it's important enough, sure.

Here's the patch I checked in.

2011-05-13  Doug Evans  <dje@google.com>

        * linux-nat.c (debug_linux_nat_async): Delete.
        Replace all references to use debug_linux_nat instead.
        (show_debug_linux_nat_async): Delete.
        (sigchld_handler): Call ui_file_write_async_safe instead of
        fprintf_unfiltered.
        (_initialize_linux_nat): Remove `set debug lin-lwp-async'.
        * ui-file.c (struct ui_file): New member to_write_async_safe.
        (null_file_write_async_safe): New function.
        (ui_file_write_async_safe): New function.
        (set_ui_file_write_async_safe): New function.
        (ui_file_new): Initialize to_write_async_safe.
        (stdio_file_write_async_safe): New function.
        (struct stdio_file): New member fd.
        (stdio_file_new): Initialize to_write_async_safe, fd.
        (stdio_file_read, stdio_file_isatty): New stdio->fd instead of
calling fileno.
        * ui-file.h (ui_file_write_async_safe_ftype): New typedef.
        (set_ui_file_write_async_safe): Declare.
        (ui_file_write_async_safe): Declare.

        doc/
        * gdb.texinfo (Completion): Update example.
        (Debugging Output): Delete `set/show debug lin-lwp-async'.

[-- Attachment #2: gdb-110513-debug-lin-lwp-async-2.patch.txt --]
[-- Type: text/plain, Size: 12349 bytes --]

2011-05-13  Doug Evans  <dje@google.com>

	* linux-nat.c (debug_linux_nat_async): Delete.
	Replace all references to use debug_linux_nat instead.
	(show_debug_linux_nat_async): Delete.
	(sigchld_handler): Call ui_file_write_async_safe instead of
	fprintf_unfiltered.
	(_initialize_linux_nat): Remove `set debug lin-lwp-async'.
	* ui-file.c (struct ui_file): New member to_write_async_safe.
	(null_file_write_async_safe): New function.
	(ui_file_write_async_safe): New function.
	(set_ui_file_write_async_safe): New function.
	(ui_file_new): Initialize to_write_async_safe.
	(stdio_file_write_async_safe): New function.
	(struct stdio_file): New member fd.
	(stdio_file_new): Initialize to_write_async_safe, fd.
	(stdio_file_read, stdio_file_isatty): New stdio->fd instead of calling fileno.
	* ui-file.h (ui_file_write_async_safe_ftype): New typedef.
	(set_ui_file_write_async_safe): Declare.
	(ui_file_write_async_safe): Declare.

	doc/
	* gdb.texinfo (Completion): Update example.
	(Debugging Output): Delete `set/show debug lin-lwp-async'.

Index: linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-nat.c,v
retrieving revision 1.203
diff -u -p -r1.203 linux-nat.c
--- linux-nat.c	9 May 2011 18:43:56 -0000	1.203
+++ linux-nat.c	13 May 2011 17:19:31 -0000
@@ -198,16 +198,6 @@ show_debug_linux_nat (struct ui_file *fi
 		    value);
 }
 
-static int debug_linux_nat_async = 0;
-static void
-show_debug_linux_nat_async (struct ui_file *file, int from_tty,
-			    struct cmd_list_element *c, const char *value)
-{
-  fprintf_filtered (file,
-		    _("Debugging of GNU/Linux async lwp module is %s.\n"),
-		    value);
-}
-
 static int disable_randomization = 1;
 
 static void
@@ -3260,7 +3250,7 @@ linux_nat_wait_1 (struct target_ops *ops
   int status = 0;
   pid_t pid;
 
-  if (debug_linux_nat_async)
+  if (debug_linux_nat)
     fprintf_unfiltered (gdb_stdlog, "LLW: enter\n");
 
   /* The first time we get here after starting a new inferior, we may
@@ -3303,7 +3293,7 @@ retry:
     {
       ourstatus->kind = TARGET_WAITKIND_IGNORE;
 
-      if (debug_linux_nat_async)
+      if (debug_linux_nat)
 	fprintf_unfiltered (gdb_stdlog, "LLW: exit (no resumed LWP)\n");
 
       restore_child_signals_mask (&prev_mask);
@@ -3546,7 +3536,7 @@ retry:
 		  /* No interesting event.  */
 		  ourstatus->kind = TARGET_WAITKIND_IGNORE;
 
-		  if (debug_linux_nat_async)
+		  if (debug_linux_nat)
 		    fprintf_unfiltered (gdb_stdlog, "LLW: exit (ignore)\n");
 
 		  restore_child_signals_mask (&prev_mask);
@@ -3561,7 +3551,7 @@ retry:
 	  /* No interesting event for PID yet.  */
 	  ourstatus->kind = TARGET_WAITKIND_IGNORE;
 
-	  if (debug_linux_nat_async)
+	  if (debug_linux_nat)
 	    fprintf_unfiltered (gdb_stdlog, "LLW: exit (ignore)\n");
 
 	  restore_child_signals_mask (&prev_mask);
@@ -3690,7 +3680,7 @@ retry:
   else
     store_waitstatus (ourstatus, status);
 
-  if (debug_linux_nat_async)
+  if (debug_linux_nat)
     fprintf_unfiltered (gdb_stdlog, "LLW: exit\n");
 
   restore_child_signals_mask (&prev_mask);
@@ -5416,8 +5406,9 @@ sigchld_handler (int signo)
 {
   int old_errno = errno;
 
-  if (debug_linux_nat_async)
-    fprintf_unfiltered (gdb_stdlog, "sigchld\n");
+  if (debug_linux_nat)
+    ui_file_write_async_safe (gdb_stdlog,
+			      "sigchld\n", sizeof ("sigchld\n") - 1);
 
   if (signo == SIGCHLD
       && linux_nat_event_pipe[0] != -1)
@@ -5801,15 +5792,6 @@ Enables printf debugging output."),
 			    show_debug_linux_nat,
 			    &setdebuglist, &showdebuglist);
 
-  add_setshow_zinteger_cmd ("lin-lwp-async", class_maintenance,
-			    &debug_linux_nat_async, _("\
-Set debugging of GNU/Linux async lwp module."), _("\
-Show debugging of GNU/Linux async lwp module."), _("\
-Enables printf debugging output."),
-			    NULL,
-			    show_debug_linux_nat_async,
-			    &setdebuglist, &showdebuglist);
-
   /* Save this mask as the default.  */
   sigprocmask (SIG_SETMASK, NULL, &normal_mask);
 
Index: ui-file.c
===================================================================
RCS file: /cvs/src/src/gdb/ui-file.c,v
retrieving revision 1.26
diff -u -p -r1.26 ui-file.c
--- ui-file.c	12 Jan 2011 01:23:28 -0000	1.26
+++ ui-file.c	13 May 2011 17:19:31 -0000
@@ -30,6 +30,7 @@
 
 static ui_file_isatty_ftype null_file_isatty;
 static ui_file_write_ftype null_file_write;
+static ui_file_write_ftype null_file_write_async_safe;
 static ui_file_fputs_ftype null_file_fputs;
 static ui_file_read_ftype null_file_read;
 static ui_file_flush_ftype null_file_flush;
@@ -42,6 +43,7 @@ struct ui_file
     int *magic;
     ui_file_flush_ftype *to_flush;
     ui_file_write_ftype *to_write;
+    ui_file_write_async_safe_ftype *to_write_async_safe;
     ui_file_fputs_ftype *to_fputs;
     ui_file_read_ftype *to_read;
     ui_file_delete_ftype *to_delete;
@@ -61,6 +63,7 @@ ui_file_new (void)
   set_ui_file_data (file, NULL, null_file_delete);
   set_ui_file_flush (file, null_file_flush);
   set_ui_file_write (file, null_file_write);
+  set_ui_file_write_async_safe (file, null_file_write_async_safe);
   set_ui_file_fputs (file, null_file_fputs);
   set_ui_file_read (file, null_file_read);
   set_ui_file_isatty (file, null_file_isatty);
@@ -155,6 +158,14 @@ null_file_fputs (const char *buf, struct
 }
 
 static void
+null_file_write_async_safe (struct ui_file *file,
+			    const char *buf,
+			    long sizeof_buf)
+{
+  return;
+}
+
+static void
 null_file_delete (struct ui_file *file)
 {
   return;
@@ -203,6 +214,14 @@ ui_file_write (struct ui_file *file,
   file->to_write (file, buf, length_buf);
 }
 
+void
+ui_file_write_async_safe (struct ui_file *file,
+			  const char *buf,
+			  long length_buf)
+{
+  file->to_write_async_safe (file, buf, length_buf);
+}
+
 long
 ui_file_read (struct ui_file *file, char *buf, long length_buf)
 {
@@ -247,6 +266,13 @@ set_ui_file_write (struct ui_file *file,
 }
 
 void
+set_ui_file_write_async_safe (struct ui_file *file,
+			      ui_file_write_async_safe_ftype *write_async_safe)
+{
+  file->to_write_async_safe = write_async_safe;
+}
+
+void
 set_ui_file_read (struct ui_file *file, ui_file_read_ftype *read)
 {
   file->to_read = read;
@@ -437,6 +463,7 @@ mem_file_write (struct ui_file *file,
    <stdio.h>'s FILE.  */
 
 static ui_file_write_ftype stdio_file_write;
+static ui_file_write_async_safe_ftype stdio_file_write_async_safe;
 static ui_file_fputs_ftype stdio_file_fputs;
 static ui_file_read_ftype stdio_file_read;
 static ui_file_isatty_ftype stdio_file_isatty;
@@ -450,6 +477,9 @@ struct stdio_file
   {
     int *magic;
     FILE *file;
+    /* The associated file descriptor is extracted ahead of time for
+       stdio_file_write_async_safe's benefit, in case fileno isn't async-safe.  */
+    int fd;
     int close_p;
   };
 
@@ -461,10 +491,12 @@ stdio_file_new (FILE *file, int close_p)
 
   stdio->magic = &stdio_file_magic;
   stdio->file = file;
+  stdio->fd = fileno (file);
   stdio->close_p = close_p;
   set_ui_file_data (ui_file, stdio, stdio_file_delete);
   set_ui_file_flush (ui_file, stdio_file_flush);
   set_ui_file_write (ui_file, stdio_file_write);
+  set_ui_file_write_async_safe (ui_file, stdio_file_write_async_safe);
   set_ui_file_fputs (ui_file, stdio_file_fputs);
   set_ui_file_read (ui_file, stdio_file_read);
   set_ui_file_isatty (ui_file, stdio_file_isatty);
@@ -510,16 +542,14 @@ stdio_file_read (struct ui_file *file, c
      the file.  Wait until at least one byte of data is available.
      Control-C can interrupt gdb_select, but not read.  */
   {
-    int fd = fileno (stdio->file);
-
     fd_set readfds;
     FD_ZERO (&readfds);
-    FD_SET (fd, &readfds);
-    if (gdb_select (fd + 1, &readfds, NULL, NULL, NULL) == -1)
+    FD_SET (stdio->fd, &readfds);
+    if (gdb_select (stdio->fd + 1, &readfds, NULL, NULL, NULL) == -1)
       return -1;
   }
 
-  return read (fileno (stdio->file), buf, length_buf);
+  return read (stdio->fd, buf, length_buf);
 }
 
 static void
@@ -536,6 +566,24 @@ stdio_file_write (struct ui_file *file, 
 }
 
 static void
+stdio_file_write_async_safe (struct ui_file *file,
+			     const char *buf, long length_buf)
+{
+  struct stdio_file *stdio = ui_file_data (file);
+
+  if (stdio->magic != &stdio_file_magic)
+    {
+      /* gettext isn't necessarily async safe, so we can't use _("error message") here.
+	 We could extract the correct translation ahead of time, but this is an extremely
+	 rare event, and one of the other stdio_file_* routines will presumably catch
+	 the problem anyway.  For now keep it simple and ignore the error here.  */
+      return;
+    }
+
+  write (stdio->fd, buf, length_buf);
+}
+
+static void
 stdio_file_fputs (const char *linebuffer, struct ui_file *file)
 {
   struct stdio_file *stdio = ui_file_data (file);
@@ -556,7 +604,7 @@ stdio_file_isatty (struct ui_file *file)
   if (stdio->magic != &stdio_file_magic)
     internal_error (__FILE__, __LINE__,
 		    _("stdio_file_isatty: bad magic number"));
-  return (isatty (fileno (stdio->file)));
+  return (isatty (stdio->fd));
 }
 
 /* Like fdopen().  Create a ui_file from a previously opened FILE.  */
Index: ui-file.h
===================================================================
RCS file: /cvs/src/src/gdb/ui-file.h,v
retrieving revision 1.16
diff -u -p -r1.16 ui-file.h
--- ui-file.h	27 Feb 2011 16:25:37 -0000	1.16
+++ ui-file.h	13 May 2011 17:19:31 -0000
@@ -45,6 +45,17 @@ typedef void (ui_file_fputs_ftype) (cons
 extern void set_ui_file_fputs (struct ui_file *stream,
 			       ui_file_fputs_ftype *fputs);
 
+/* This version of "write" is safe for use in signal handlers.
+   It's not guaranteed that all existing output will have been
+   flushed first.
+   Implementations are also free to ignore some or all of the request.
+   fputs_async is not provided as the async versions are rarely used,
+   no point in having both for a rarely used interface.  */
+typedef void (ui_file_write_async_safe_ftype)
+  (struct ui_file *stream, const char *buf, long length_buf);
+extern void set_ui_file_write_async_safe
+  (struct ui_file *stream, ui_file_write_async_safe_ftype *write_async_safe);
+
 typedef long (ui_file_read_ftype) (struct ui_file *stream,
 				   char *buf, long length_buf);
 extern void set_ui_file_read (struct ui_file *stream,
@@ -83,6 +94,9 @@ extern int ui_file_isatty (struct ui_fil
 extern void ui_file_write (struct ui_file *file, const char *buf,
 			   long length_buf);
 
+extern void ui_file_write_async_safe (struct ui_file *file, const char *buf,
+				      long length_buf);
+
 /* NOTE: copies left to right.  */
 extern void ui_file_put (struct ui_file *src,
 			 ui_file_put_method_ftype *write, void *dest);
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.834
diff -u -p -r1.834 gdb.texinfo
--- doc/gdb.texinfo	12 May 2011 12:09:16 -0000	1.834
+++ doc/gdb.texinfo	13 May 2011 17:19:32 -0000
@@ -1592,8 +1592,10 @@ left-hand-side:
 
 @smallexample
 (@value{GDBP}) p gdb_stdout.@kbd{M-?}
-magic      to_delete  to_fputs   to_put     to_rewind  
-to_data    to_flush   to_isatty  to_read    to_write   
+magic                to_fputs             to_rewind
+to_data              to_isatty            to_write
+to_delete            to_put               to_write_async_safe
+to_flush             to_read
 @end smallexample
 
 @noindent
@@ -1607,6 +1609,7 @@ struct ui_file
    int *magic;
    ui_file_flush_ftype *to_flush;
    ui_file_write_ftype *to_write;
+   ui_file_write_async_safe_ftype *to_write_async_safe;
    ui_file_fputs_ftype *to_fputs;
    ui_file_read_ftype *to_read;
    ui_file_delete_ftype *to_delete;
@@ -20071,12 +20074,6 @@ Displays the current state of @value{GDB
 Turns on or off debugging messages from the Linux LWP debug support.
 @item show debug lin-lwp
 Show the current state of Linux LWP debugging messages.
-@item set debug lin-lwp-async
-@cindex @sc{gnu}/Linux LWP async debug messages
-@cindex Linux lightweight processes
-Turns on or off debugging messages from the Linux LWP async debug support.
-@item show debug lin-lwp-async
-Show the current state of Linux LWP async debugging messages.
 @item set debug observer
 @cindex observer debugging info
 Turns on or off display of @value{GDBN} observer debugging.  This

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

* Re: [RFA] Remove set debug lin-lwp-async, new ui_file.to_write_async_safe
  2011-05-13 17:32         ` Doug Evans
@ 2011-05-13 19:05           ` Stan Shebs
  2011-05-14  5:45             ` Doug Evans
  0 siblings, 1 reply; 12+ messages in thread
From: Stan Shebs @ 2011-05-13 19:05 UTC (permalink / raw)
  To: gdb-patches

On 5/13/11 10:32 AM, Doug Evans wrote:
>          (stdio_file_write_async_safe): New function.

FYI, my build moans about the unchecked result of write() in this, 
although it's not clear what one ought to do with a result.

Stan
stan@codesourcery.com


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

* Re: [RFA] Remove set debug lin-lwp-async, new ui_file.to_write_async_safe
  2011-05-13 19:05           ` Stan Shebs
@ 2011-05-14  5:45             ` Doug Evans
  0 siblings, 0 replies; 12+ messages in thread
From: Doug Evans @ 2011-05-14  5:45 UTC (permalink / raw)
  To: Stan Shebs; +Cc: gdb-patches

On Fri, May 13, 2011 at 12:05 PM, Stan Shebs <stanshebs@earthlink.net> wrote:
> On 5/13/11 10:32 AM, Doug Evans wrote:
>>
>>         (stdio_file_write_async_safe): New function.
>
> FYI, my build moans about the unchecked result of write() in this, although
> it's not clear what one ought to do with a result.

Yeah.  The gcc I was using doesn't warn about this.
Hui Zhu checked in a fix using the same style as the rest of the file
(which was the right thing to do, at least at the moment).

I added a comment because

  if (foo ())
    ;

is so not what I expect to avoid warnings of this kind.
And it looks like the gcc gods decided (void) foo (); wasn't ok.  Sigh.

I didn't add comments to the other invocations, but I wouldn't mind them.


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

end of thread, other threads:[~2011-05-14  5:45 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-10 20:47 [RFA] Remove set debug lin-lwp-async, new ui_file.to_write_async_safe Doug Evans
2011-05-10 20:51 ` Doug Evans
2011-05-10 21:30 ` Pedro Alves
2011-05-10 22:19   ` Doug Evans
2011-05-11  7:39     ` Jan Kratochvil
2011-05-11 15:50       ` Doug Evans
2011-05-13 17:32         ` Doug Evans
2011-05-13 19:05           ` Stan Shebs
2011-05-14  5:45             ` Doug Evans
2011-05-10 21:38 ` Pedro Alves
2011-05-10 22:20   ` Doug Evans
2011-05-11  2:50 ` Eli Zaretskii

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox