Re: [RFC PATCH 1/7] gdbsupport: Add an event-pipe class.

[Pedro Alves <pedro@palves.net>]

On 2021-06-07 6:09 p.m., John Baldwin wrote:
> This pulls out the implementation of an event pipe used to implement
> target async support in both linux-low.cc (gdbserver) and linux-nat.c
> (gdb).
> 

Seems like a good idea, thanks.

> +  /* Create a new pipe.  */
> +  bool open ();
> +
> +  /* Close the pipe.  */
> +  void close ();
> +
> +  /* True if the event pipe has been initialized.  */
> +  bool active () const { return fds[0] != -1; }
> +

Did you consider "is_open" instead of "active"?  I think reading "active()" at the call
sites can make one wonder whether "active" is a different state from being
open, like e.g., the pipe is open but not registered in the event loop.

> diff --git a/gdbsupport/event-pipe.h b/gdbsupport/event-pipe.h
> new file mode 100644
> index 0000000000..1717ea6790
> --- /dev/null
> +++ b/gdbsupport/event-pipe.h
> @@ -0,0 +1,55 @@
> +/* Event pipe for GDB, the GNU debugger.
> +
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef COMMON_EVENT_PIPE_H
> +#define COMMON_EVENT_PIPE_H
> +
> +/* An event pipe used as a waitable file in the event loop in place of
> +   some other event not associated with a file descriptor.  */

Note this comment can be a little bit unclear given we also have
create_async_event_handler / mark_async_event_handler, for events not associated
with a file descriptor.

We need to use a pipe iff we need to wake up the select/poll in the event loop
from a signal handler, otherwise we can use the cheaper mark_async_event_handler
from mainline code.

Maybe mention in the comment that this is used for implementing the well-known
self-pipe trick?