Re: [PATCH] vhost-user: rewrite vu_dispatch with if-else

[Michael S. Tsirkin]

On Sun, Aug 04, 2024 at 10:23:53PM +0800, luzhixing12345 wrote:
> rewrite with if-else instead of goto
> 
> and I have a question, in two incorrent cases
> 
> - need reply but no reply_requested
> - no need reply but has reply_requested
> 
> should we call vu_panic or print warning message?

this is not how you post a patch to the list.


> ---
>  subprojects/libvhost-user/libvhost-user.c | 39 +++++++++++++----------
>  subprojects/libvhost-user/libvhost-user.h |  6 ++--
>  2 files changed, 27 insertions(+), 18 deletions(-)
> 
> diff --git a/subprojects/libvhost-user/libvhost-user.c 
> b/subprojects/libvhost-user/libvhost-user.c
> index 9c630c2170..187e25f9bb 100644
> --- a/subprojects/libvhost-user/libvhost-user.c
> +++ b/subprojects/libvhost-user/libvhost-user.c
> @@ -2158,32 +2158,39 @@ vu_dispatch(VuDev *dev)
>  {
>      VhostUserMsg vmsg = { 0, };
>      int reply_requested;
> -    bool need_reply, success = false;
> +    bool need_reply, success = true;
>  
>      if (!dev->read_msg(dev, dev->sock, &vmsg)) {
> -        goto end;
> +        success = false;
> +        free(vmsg.data);
> +        return success;
>      }
>  
>      need_reply = vmsg.flags & VHOST_USER_NEED_REPLY_MASK;
>  
>      reply_requested = vu_process_message(dev, &vmsg);
> -    if (!reply_requested && need_reply) {
> -        vmsg_set_reply_u64(&vmsg, 0);
> -        reply_requested = 1;
> -    }
> -
> -    if (!reply_requested) {
> -        success = true;
> -        goto end;
> -    }
>  
> -    if (!vu_send_reply(dev, dev->sock, &vmsg)) {
> -        goto end;
> +    if (need_reply) {
> +        if (reply_requested) {
> +            if (!vu_send_reply(dev, dev->sock, &vmsg)) {
> +                success = false;
> +            }
> +        } else {
> +            // need reply but no reply requested, return 0(u64)
> +            vmsg_set_reply_u64(&vmsg, 0);
> +            if (!vu_send_reply(dev, dev->sock, &vmsg)) {
> +                success = false;
> +            }
> +        }
> +    } else {
> +        // no need reply but reply requested, send a reply
> +        if (reply_requested) {
> +            if (!vu_send_reply(dev, dev->sock, &vmsg)) {
> +                success = false;
> +            }
> +        }
>      }
>  
> -    success = true;
> -
> -end:
>      free(vmsg.data);
>      return success;
>  }
> diff --git a/subprojects/libvhost-user/libvhost-user.h 
> b/subprojects/libvhost-user/libvhost-user.h
> index deb40e77b3..2daf8578f6 100644
> --- a/subprojects/libvhost-user/libvhost-user.h
> +++ b/subprojects/libvhost-user/libvhost-user.h
> @@ -238,6 +238,8 @@ typedef struct VuDev VuDev;
>  
>  typedef uint64_t (*vu_get_features_cb) (VuDev *dev);
>  typedef void (*vu_set_features_cb) (VuDev *dev, uint64_t features);
> +typedef uint64_t (*vu_get_protocol_features_cb) (VuDev *dev);
> +typedef void (*vu_set_protocol_features_cb) (VuDev *dev, uint64_t features);
>  typedef int (*vu_process_msg_cb) (VuDev *dev, VhostUserMsg *vmsg,
>                                    int *do_reply);
>  typedef bool (*vu_read_msg_cb) (VuDev *dev, int sock, VhostUserMsg *vmsg);
> @@ -256,9 +258,9 @@ typedef struct VuDevIface {
>      vu_set_features_cb set_features;
>      /* get the protocol feature bitmask from the underlying vhost
>       * implementation */
> -    vu_get_features_cb get_protocol_features;
> +    vu_get_protocol_features_cb get_protocol_features;
>      /* enable protocol features in the underlying vhost implementation. */
> -    vu_set_features_cb set_protocol_features;
> +    vu_set_protocol_features_cb set_protocol_features;
>      /* process_msg is called for each vhost-user message received */
>      /* skip libvhost-user processing if return value != 0 */
>      vu_process_msg_cb process_msg;
> -- 
> 2.34.1