Re: [Minios-devel] [UNIKRAFT LWIP PATCH 5/7] Change the return of errors to match vfs expectations

[Costin Lupu <costin.lup@xxxxxxxxx>]

Hi Mihai,

A subject suggestion would be "sockets.c: Change error handling to match
vfs expectations" (btw, using path prefixes, such as 'sockets.c: ',
helps in limiting the scope of the fix, i.e. one can understanding that
it fixes only in the given file).

The commit message shouldn't be empty and say something about the vfs
expectations. I know you said it in the comments, but it helps putting
it in the commit message as well because it is the first thing one sees
one reading/reviewing the patch.

On 6/26/19 4:54 AM, Mihai Pogonaru wrote:
> Signed-off-by: Mihai Pogonaru <pogonarumihai@xxxxxxxxx>
> ---
>  sockets.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/sockets.c b/sockets.c
> index 1a3ddfd..38639f8 100644
> --- a/sockets.c
> +++ b/sockets.c
> @@ -237,9 +237,18 @@ static int sock_net_close(struct vnode *s_vnode,
>       /*
>        * Free socket file
>        * The rest of the resources will be freed by vfs
> +      *
> +      * TODO: vfs ignores close errors right now, so free our file
>        */
>       uk_free(uk_alloc_get_default(), file);
>  
> +     /*
> +      * lwip sets errno and returns -1 in case of error, but
> +      * vfs expects us to return a positive errno
> +      */
> +     if (ret < 0)
> +             return errno;
> +
>       return ret;
>  }
>  
> @@ -255,9 +264,12 @@ static int sock_net_write(struct vnode *s_vnode,
>                                   file->vfscore_file->fd,
>                                   file->sock_fd));
>       ret = lwip_writev(file->sock_fd, buf->uio_iov, buf->uio_iovcnt);
> -     /* lwip sets errno and returns -1 in case of error */
> +     /*
> +      * lwip sets errno and returns -1 in case of error, but
> +      * vfs expects us to return a positive errno
> +      */
>       if (ret < 0)
> -             return ret;
> +             return errno;
>  
>       buf->uio_resid -= ret;
>       return 0;
> @@ -276,9 +288,12 @@ static int sock_net_read(struct vnode *s_vnode,
>                                   file->vfscore_file->fd,
>                                   file->sock_fd));
>       ret = lwip_readv(file->sock_fd, buf->uio_iov, buf->uio_iovcnt);
> -     /* lwip sets errno and returns -1 in case of error */
> +     /*
> +      * lwip sets errno and returns -1 in case of error, but
> +      * vfs expects us to return a positive errno
> +      */
>       if (ret < 0)
> -             return ret;
> +             return errno;
>  
>       buf->uio_resid -= ret;
>       return 0;
> 

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel