iter_file_splice_write() return value (was: Re: ->splice_write() via ->write_iter())

[Geert Uytterhoeven]

Hi Al,

On Thu, Jun 12, 2014 at 8:14 PM, Linux Kernel Mailing List
<linux-kernel@xxxxxxxxxxxxxxx> wrote:
> Gitweb:     http://git.kernel.org/linus/;a=commit;h=8d0207652cbe27d1f962050737848e5ad4671958
> Commit:     8d0207652cbe27d1f962050737848e5ad4671958

>     ->splice_write() via ->write_iter()
>
>     iter_file_splice_write() - a ->splice_write() instance that gathers the
>     pipe buffers, builds a bio_vec-based iov_iter covering those and feeds
>     it to ->write_iter().  A bunch of simple cases coverted to that...
>
>     [AV: fixed the braino spotted by Cyrill]
>
>     Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>

> @@ -1052,6 +1053,145 @@ generic_file_splice_write(struct pipe_inode_info *pipe, struct file *out,
>
>  EXPORT_SYMBOL(generic_file_splice_write);
>
> +/**
> + * iter_file_splice_write - splice data from a pipe to a file
> + * @pipe:      pipe info
> + * @out:       file to write to
> + * @ppos:      position in @out
> + * @len:       number of bytes to splice
> + * @flags:     splice modifier flags
> + *
> + * Description:
> + *    Will either move or copy pages (determined by @flags options) from
> + *    the given pipe inode to the given file.
> + *    This one is ->write_iter-based.

What's the meaning of the return value? This is not documented.

> +ssize_t
> +iter_file_splice_write(struct pipe_inode_info *pipe, struct file *out,
> +                         loff_t *ppos, size_t len, unsigned int flags)
> +{
> +       struct splice_desc sd = {
> +               .total_len = len,
> +               .flags = flags,
> +               .pos = *ppos,
> +               .u.file = out,
> +       };
> +       int nbufs = pipe->buffers;
> +       struct bio_vec *array = kcalloc(nbufs, sizeof(struct bio_vec),
> +                                       GFP_KERNEL);
> +       ssize_t ret;

fs/splice.c:954: warning: âretâ may be used uninitialized in this function

> +
> +       if (unlikely(!array))
> +               return -ENOMEM;
> +
> +       pipe_lock(pipe);
> +
> +       splice_from_pipe_begin(&sd);
> +       while (sd.total_len) {

If the "len" parameter is zero, this loop will be skipped, and "ret" will
be uninitialized.

What should it return? 0? -EINVAL?

> +       }
> +done:
> +       kfree(array);
> +       splice_from_pipe_end(pipe, &sd);
> +
> +       pipe_unlock(pipe);
> +
> +       if (sd.num_spliced)
> +               ret = sd.num_spliced;
> +
> +       return ret;
> +}

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/