Re: [PATCH v2 17/35] xen/console: introduce consoled_is_enabled()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.12.2024 05:41, Denis Mukhin via B4 Relay wrote:
> --- a/xen/drivers/char/consoled.c
> +++ b/xen/drivers/char/consoled.c
> @@ -43,13 +43,13 @@ struct xencons_interface *consoled_get_ring_addr(void)
>  static char buf[BUF_SZ + 1];
>  
>  /* Receives characters from a domain's PV console */
> -void consoled_guest_rx(void)
> +int consoled_guest_rx(void)
>  {
>      size_t idx = 0;
>      XENCONS_RING_IDX cons, prod;
>  
>      if ( !cons_ring )
> -        return;
> +        return 0;
>  
>      spin_lock(&rx_lock);
>  
> @@ -91,15 +91,17 @@ void consoled_guest_rx(void)
>  
>   out:
>      spin_unlock(&rx_lock);
> +
> +    return 0;
>  }
>  
>  /* Sends a character into a domain's PV console */
> -void consoled_guest_tx(char c)
> +int consoled_guest_tx(char c)
>  {
>      XENCONS_RING_IDX cons, prod;
>  
>      if ( !cons_ring )
> -        return;
> +        return 0;
>  
>      cons = ACCESS_ONCE(cons_ring->in_cons);
>      prod = cons_ring->in_prod;
> @@ -118,6 +120,7 @@ void consoled_guest_tx(char c)
>  
>      cons_ring->in[MASK_XENCONS_IDX(prod++, cons_ring->in)] = c;
>  
> +
>      /* Write to the ring before updating the pointer */

No excess blank lines please.

> @@ -125,6 +128,13 @@ void consoled_guest_tx(char c)
>   notify:
>      /* Always notify the guest: prevents receive path from getting stuck. */
>      pv_shim_inject_evtchn(pv_console_evtchn());
> +
> +    return 0;
> +}

For both of the functions - what use is it to make the functions return
a value, when all they'd ever return is zero (and callers don't care)?
I'm also having a hard time seeing how this adjustment is related to ...

> +bool consoled_is_enabled(void)
> +{
> +    return pv_shim && pv_console;
>  }

... the introduction of this function (which by itself is probably fine).

Jan