Re: [PATCH] gdb: add a file event mask parameter to add_file_handler

[Patrick Monnerat <patrick@monnerat.net>]

On 8/30/22 19:34, Tom Tromey wrote:
> Patrick> Yes. In Insight, command input from stdin is disabled by setting
> Patrick> main_ui->input_fd to -1. A later call to register_file_handler from
> Patrick> async_enable_stdin causes add_file_handler to be called with fd =
> Patrick> -1. Maybe I should add a comment for this.
>
> Tom> I took this same approach in my DAP implementation, which is why I
> Tom> changed gdb to always call the 'ui' methods to manipulate the event
> Tom> handlers.  In my patch I then change these methods to check for the -1
> Tom> case.
>
> Tom> I can submit this bit separately next week.
> Tom> Or you can do it, it's trivial.
>
> What do you think of this?

This is great! I applied your changes to Insight and dropped the fd<0 
test in add_file_handler with total success.

I'll soon submit version 2 of my patch with the fd<0 test removed, and 
an updated test for mask==0 (this case should call delete_file_handler).

This would have to be pushed after your patch, of course!

Thanks a lot.

Patrick

> Tom
>
> commit 73d104a6fc246f0cd7d42a4ed1c009304fb55327
> Author: Tom Tromey <tromey@adacore.com>
> Date:   Tue Aug 30 11:30:13 2022 -0600
>
>      Let ui::input_fd be -1
>      
>      This changes gdb so that, if ui::input_fd is set to -1, then it will
>      not be registered with the event loop.  This is useful for the DAP
>      support code I wrote, but as it turns out to also be useful to
>      Insight, it seems best to check it in separately.
>
> diff --git a/gdb/event-top.c b/gdb/event-top.c
> index 02b3786320f..4547d614522 100644
> --- a/gdb/event-top.c
> +++ b/gdb/event-top.c
> @@ -533,8 +533,9 @@ stdin_event_handler (int error, gdb_client_data client_data)
>   void
>   ui::register_file_handler ()
>   {
> -  add_file_handler (input_fd, stdin_event_handler, this,
> -		    string_printf ("ui-%d", num), true);
> +  if (input_fd != -1)
> +    add_file_handler (input_fd, stdin_event_handler, this,
> +		      string_printf ("ui-%d", num), true);
>   }
>   
>   /* See top.h.  */
> @@ -542,7 +543,8 @@ ui::register_file_handler ()
>   void
>   ui::unregister_file_handler ()
>   {
> -  delete_file_handler (input_fd);
> +  if (input_fd != -1)
> +    delete_file_handler (input_fd);
>   }
>   
>   /* Re-enable stdin after the end of an execution command in
> diff --git a/gdb/top.h b/gdb/top.h
> index 5c1db84b2ce..9ea07262c39 100644
> --- a/gdb/top.h
> +++ b/gdb/top.h
> @@ -118,7 +118,8 @@ struct ui
>     FILE *errstream;
>   
>     /* The file descriptor for the input stream, so that we can register
> -     it with the event loop.  */
> +     it with the event loop.  This can be set to -1 to prevent this
> +     registration.  */
>     int input_fd;
>   
>     /* Whether ISATTY returns true on input_fd.  Cached here because