Re: [Xen-devel] [RFC 02/19] xen: guestcopy: Provide an helper to copy string from guest

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.06.14 at 18:17, <julien.grall@xxxxxxxxxx> wrote:

While generally I'm okay with adding such a helper, it should be done
a little more cleanly I think:

> --- /dev/null
> +++ b/xen/common/guestcopy.c
> @@ -0,0 +1,28 @@
> +#include <xen/config.h>
> +#include <xen/lib.h>
> +#include <xen/guest_access.h>
> +
> +int copy_string_from_guest(XEN_GUEST_HANDLE(char) u_buf, char **buf,
> +                           unsigned long size, unsigned long max_size)

Both of these ought to be size_t (as it was in the flask original).

> +{
> +    char *tmp;
> +
> +    if ( size > max_size )
> +        return -ENOENT;

ENOBUFS would seem the better error code here.

> +
> +    /* Add an extra +1 to append \0. We can't assume the guest will
> +     * provide a valid string */

Now this is the case for flask, but for a generic string copying
routine I don't think this is desirable. It seems especially wrong to
aid the guest with putting a NUL where none was. If you really
want this, I guess you would be better off adding two variants:
One which demands the string to be NUL-terminated (in which
case passing in a size is sort of bogus), and one which takes a
size and inserts a NUL.

And in the end with the above I guess you realize why flask
rolled its own special purpose function rather than adding a
generic helper.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel