Re: [Xen-devel] [PATCH 3/4] xen/public: Document HYPERCALL_console_io()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 02.04.19 at 18:42, <julien.grall@xxxxxxx> wrote:
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -97,6 +97,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
>  #define __HYPERVISOR_set_timer_op         15
>  #define __HYPERVISOR_event_channel_op_compat 16 /* compat since 0x00030202 */
>  #define __HYPERVISOR_xen_version          17
> +/* HYPERVISOR_console_io(int cmd, int count, XEN_GUEST_HANDLE(char) buffer); 
> */
>  #define __HYPERVISOR_console_io           18

I pretty strongly object to this comment going where you put it.
I'd be fine if you put it ...

> @@ -486,7 +487,25 @@ DEFINE_XEN_GUEST_HANDLE(mmuext_op_t);
>  /* ` } */
>  
>  /*
> - * Commands to HYPERVISOR_console_io().
> + * Commands to HYPERVISOR_console_io()

... into here, just like done for HYPERVISOR_mmuext_op() and
HYPERVISOR_update_va_mapping*() immediately above. This
would then also help ...

> + * @cmd: Command (see below)
> + * @count: Size of the buffer to read/write
> + * @buffer: Pointer in the guest memory

... associating these names.

Also please note the quotation used by the mentioned
existing doc comments, as well as a few other formal aspects
(like them also making clear what the return type is). I think
that's a model used elsewhere as well, so I think you should
follow it here.

The other thing is: As mentioned elsewhere, I don't think the
first two parameters should be plain int. I'm not happy to see
this proliferate into documentation as well, i.e. I'd prefer if
this was corrected before adding documentation. Would you
be willing to do this, or should I add it to my todo list?

Jan



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