Re: [PATCH 1/4] virtiofsd: Remove fuse_req_getgroups

[Philippe Mathieu-Daudé]

On 2/4/20 12:04 PM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <address@hidden>
>
> Remove fuse_req_getgroups that's unused in virtiofsd; it came in
> from libfuse but we don't actually use it.  It was called from
> fuse_getgroups which we previously removed (but had left it's header
> in).
>
> Coverity had complained about null termination in it, but removing
> it is the easiest answer.

:)

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

> Fixes: Coverity CID: 1413117 (String not null terminated)
> Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> ---
>   tools/virtiofsd/fuse.h          | 20 ---------
>   tools/virtiofsd/fuse_lowlevel.c | 77 ---------------------------------
>   tools/virtiofsd/fuse_lowlevel.h | 21 ---------
>   3 files changed, 118 deletions(-)
>
> diff --git a/tools/virtiofsd/fuse.h b/tools/virtiofsd/fuse.h
> index 7a4c713559..aba13fef2d 100644
> --- a/tools/virtiofsd/fuse.h
> +++ b/tools/virtiofsd/fuse.h
> @@ -1006,26 +1006,6 @@ void fuse_exit(struct fuse *f);
>    */
>   struct fuse_context *fuse_get_context(void);
>   
> -/**
> - * Get the current supplementary group IDs for the current request
> - *
> - * Similar to the getgroups(2) system call, except the return value is
> - * always the total number of group IDs, even if it is larger than the
> - * specified size.
> - *
> - * The current fuse kernel module in linux (as of 2.6.30) doesn't pass
> - * the group list to userspace, hence this function needs to parse
> - * "/proc/$TID/task/$TID/status" to get the group IDs.
> - *
> - * This feature may not be supported on all operating systems.  In
> - * such a case this function will return -ENOSYS.
> - *
> - * @param size size of given array
> - * @param list array of group IDs to be filled in
> - * @return the total number of supplementary group IDs or -errno on failure
> - */
> -int fuse_getgroups(int size, gid_t list[]);
> -
>   /**
>    * Check if the current request has already been interrupted
>    *
> diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
> index de2e2e0c65..01c418aade 100644
> --- a/tools/virtiofsd/fuse_lowlevel.c
> +++ b/tools/virtiofsd/fuse_lowlevel.c
> @@ -2667,83 +2667,6 @@ int fuse_lowlevel_is_virtio(struct fuse_session *se)
>       return !!se->virtio_dev;
>   }
>   
> -#ifdef linux
> -int fuse_req_getgroups(fuse_req_t req, int size, gid_t list[])
> -{
> -    char *buf;
> -    size_t bufsize = 1024;
> -    char path[128];
> -    int ret;
> -    int fd;
> -    unsigned long pid = req->ctx.pid;
> -    char *s;
> -
> -    sprintf(path, "/proc/%lu/task/%lu/status", pid, pid);
> -
> -retry:
> -    buf = malloc(bufsize);
> -    if (buf == NULL) {
> -        return -ENOMEM;
> -    }
> -
> -    ret = -EIO;
> -    fd = open(path, O_RDONLY);
> -    if (fd == -1) {
> -        goto out_free;
> -    }
> -
> -    ret = read(fd, buf, bufsize);
> -    close(fd);
> -    if (ret < 0) {
> -        ret = -EIO;
> -        goto out_free;
> -    }
> -
> -    if ((size_t)ret == bufsize) {
> -        free(buf);
> -        bufsize *= 4;
> -        goto retry;
> -    }
> -
> -    ret = -EIO;
> -    s = strstr(buf, "\nGroups:");
> -    if (s == NULL) {
> -        goto out_free;
> -    }
> -
> -    s += 8;
> -    ret = 0;
> -    while (1) {
> -        char *end;
> -        unsigned long val = strtoul(s, &end, 0);
> -        if (end == s) {
> -            break;
> -        }
> -
> -        s = end;
> -        if (ret < size) {
> -            list[ret] = val;
> -        }
> -        ret++;
> -    }
> -
> -out_free:
> -    free(buf);
> -    return ret;
> -}
> -#else /* linux */
> -/*
> - * This is currently not implemented on other than Linux...
> - */
> -int fuse_req_getgroups(fuse_req_t req, int size, gid_t list[])
> -{
> -    (void)req;
> -    (void)size;
> -    (void)list;
> -    return -ENOSYS;
> -}
> -#endif
> -
>   void fuse_session_exit(struct fuse_session *se)
>   {
>       se->exited = 1;
> diff --git a/tools/virtiofsd/fuse_lowlevel.h b/tools/virtiofsd/fuse_lowlevel.h
> index 138041e5f1..8f6d705b5c 100644
> --- a/tools/virtiofsd/fuse_lowlevel.h
> +++ b/tools/virtiofsd/fuse_lowlevel.h
> @@ -1704,27 +1704,6 @@ void *fuse_req_userdata(fuse_req_t req);
>    */
>   const struct fuse_ctx *fuse_req_ctx(fuse_req_t req);
>   
> -/**
> - * Get the current supplementary group IDs for the specified request
> - *
> - * Similar to the getgroups(2) system call, except the return value is
> - * always the total number of group IDs, even if it is larger than the
> - * specified size.
> - *
> - * The current fuse kernel module in linux (as of 2.6.30) doesn't pass
> - * the group list to userspace, hence this function needs to parse
> - * "/proc/$TID/task/$TID/status" to get the group IDs.
> - *
> - * This feature may not be supported on all operating systems.  In
> - * such a case this function will return -ENOSYS.
> - *
> - * @param req request handle
> - * @param size size of given array
> - * @param list array of group IDs to be filled in
> - * @return the total number of supplementary group IDs or -errno on failure
> - */
> -int fuse_req_getgroups(fuse_req_t req, int size, gid_t list[]);
> -
>   /**
>    * Callback function for an interrupt
>    *