Hello,

Thanks for this change, the principle looks good.

Alperen Erkan, le dim. 20 sept. 2026 11:47:27 +0300, a ecrit:
> diff --git a/hurd/boot/boot.c b/hurd/boot/boot.c
> --- a/hurd/boot/boot.c
> +++ b/hurd/boot/boot.c
> @@ -2,6 +3,7 @@
>     as if we were the kernel.

Please take care with the text content: your mailer has mangled spaces
completely, making the patch not applicable. You can try to post it as
attached file, your mailer may then do a better job at keeping the file
exactly correct. Just make sure that your mailer attaches it as text
attachment so it shows up nicely in everybody's mailer.

>     Copyright (C) 1993,94,95,96,97,98,99,2000,01,02,2006,14,16
>       Free Software Foundation, Inc.
> +   Copyright (C) 2026 Alperen ERKAN

No, after assigning copyright, the copyright will be owned by FSF.

People credit is recorded in the git commit history.

> @@ -84,9 +125,50 @@ static int privileged;
>  static int want_privileged;
> 
>  static struct termios orig_tty_state;
> +static int termstate_initialized;
>  static int isig;
>  static char *kernel_command_line;
> 
> +static void
> +restore_termstate (void)
> +{
> +  if (! termstate_initialized)
> +    return;
> +  tcsetattr (0, 0, &orig_tty_state);
> +  termstate_initialized = 0;

But in the SIGTSTP case, on continuation we'll want to apply the tty
state that we want. Your change makes the SIGCONT handler not do that
when termstate_initialized is 0 (which is expected).

In practice, orig_tty_state really remains initialized, so better leave
termstate_initialized as true?

> +}
> +
> +static void
> +sig_handler (int sig)
> +{
> +  switch (sig)
> +    {

There is nothing shared in this function. Better use different
functions, it will be clearer in init_termstate etc. what kind of
handler we we set for each signals.

> +    case SIGCONT:
> +      /* Re-enter raw mode after being stopped.  */
> +      if (termstate_initialized)
> + {
> +  struct termios tty_state = orig_tty_state;
> +  cfmakeraw (&tty_state);
> +  if (isig)
> +    tty_state.c_lflag |= ISIG;
> +  tcsetattr (0, 0, &tty_state);
> + }

Better make init_termstate record the tty state that we want to have, to
avoid having to re-compute it.

Also, you'd want to set SIGTSTP to sig_handler again, to catch the next
terminal stop.

> +      break;
> +
> +    case SIGTSTP:
> +      restore_termstate ();
> +      signal (SIGTSTP, SIG_DFL);
> +      raise (SIGTSTP);
> +      break;
> +
> +    default:
> +      restore_termstate ();
> +      signal (sig, SIG_DFL);
> +      raise (sig);
> +      break;
> +    }
> +}
> +
>  static void
>  init_termstate (void)
>  {
> @@ -143,12 +145,14 @@ init_termstate (void)
> 
>    if (tcsetattr (0, 0, &tty_state) < 0)
>      error (11, errno, "tcsetattr");
> -}
> 
> -static void
> -restore_termstate (void)
> -{
> -  tcsetattr (0, 0, &orig_tty_state);
> +  termstate_initialized = 1;
> +
> +  atexit (restore_termstate);
> +  signal (SIGINT, sig_handler);
> +  signal (SIGTERM, sig_handler);
> +  signal (SIGTSTP, sig_handler);
> +  signal (SIGCONT, sig_handler);
>  }



> @@ -163,6 +173,16 @@ host_exit (int status)
>    exit (status);
>  }
> 
> +/* Best-effort write of a diagnostic message to stderr.  */
> +static void
> +write_diag (const char *msg, size_t len)
> +{
> +  ssize_t err;
> +  do
> +    err = write (2, msg, len);
> +  while (err < 0 && errno == EINTR);
> +}

That is unrelated for now. Better introduce it in the patch that will
use it.

> @@ -195,17 +185,7 @@ struct store *root_store;
>  pthread_spinlock_t queuelock = PTHREAD_SPINLOCK_INITIALIZER;
>  pthread_spinlock_t readlock = PTHREAD_SPINLOCK_INITIALIZER;
> 
> -mach_port_t php_child_name, psmdp_child_name, taskname;
> -
> -task_t child_task;
> -mach_port_t bootport;

There is still code that uses these two, even if commented. Either drop
that (commented) code, explaining why we don't actually need it, or keep
the variables.

The other unused variables can indeed go away.

> -
> -int console_mscount;

That will break the build.

> @@ -315,6 +326,17 @@ boot_demuxer (mach_msg_header_t *inp,
> 
>  static void read_reply (void);
>  static void * msg_thread (void *);
> +static void * select_thread (void *);
> +
> +/* Console input event handling.  The main thread polls the host stdin
> +   only while console read requests are queued; the message threads
> +   wake it via WAKE_PIPE.  */
> +static int wake_pipe[2];
> +static int select_pipe[2];
> +static _Atomic int stdin_eof;
> +
> +/* Maximum size of a single console read request (out-of-line).  */
> +#define CONSOLE_READ_MAX (16 * 1024 * 1024)

This is also unused for now, better introduce along usage.

Samuel

Reply via email to