[Virtio-fs] [PATCH v3 1/2] virtiofsd: add definition of fuse_buf_writev()

[Stefan Hajnoczi]

On Thu, Aug 08, 2019 at 02:15:27PM +0800, piaojun wrote:
> @@ -71,6 +72,42 @@ static ssize_t fuse_buf_write(const struct fuse_buf *dst, size_t dst_off,
>  	return copied;
>  }
> 
> +ssize_t fuse_buf_writev(fuse_req_t req,
> +			     struct fuse_bufvec *out_buf,
> +			     struct fuse_bufvec *in_buf,
> +			     enum fuse_buf_copy_flags flags)

FUSE_BUF_FD_SEEK is defined in enum fuse_buf_flags, not enum
fuse_buf_copy_flags.  This argument can be removed and the check below
can be changed from:

  if (flags & FUSE_BUF_FD_SEEK)

to

  if (out_buf->buf[0].flags & FUSE_BUF_FD_SEEK)

> +{
> +	ssize_t res, i, buf_index, iovcnt;
> +	struct iovec * iov;
> +	int fd = out_buf->buf[0].fd;
> +	off_t pos = out_buf->buf[0].pos;

A struct fuse_bufvec may have multiple elements but this function
assumes it only has 1.  Please use struct fuse_buf *out_buf instead.
This way it's clear that only 1 fuse_buf will be written.

> +
> +	if (in_buf->count > 2)
> +		iovcnt = in_buf->count - 1;
> +	else
> +		iovcnt = 1;
> +
> +	iov = calloc(iovcnt, sizeof(struct iovec));
> +	if (!iov)
> +		return -ENOMEM;
> +
> +	for (i = 0, buf_index = 1; i < iovcnt; i++, buf_index++) {
> +		iov[i].iov_base = in_buf->buf[buf_index].mem;
> +		iov[i].iov_len = in_buf->buf[buf_index].size;
> +	}

Why is in_buf->buf[0] is skipped?

> +
> +	if (flags & FUSE_BUF_FD_SEEK)
> +		res = pwritev(fd, iov, iovcnt, pos);

Please move off_t pos = out_buf->buf[0].pos into this if statement body
to avoid a possible uninitialized memory access when !(flags &
FUSE_BUF_FD_SEEK).  This makes valgrind and other tools happy.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virtio-fs/attachments/20190808/0d000ff9/attachment.sig>