[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



Hi Jan,

On 05/08/2019 11:07, Jan Beulich wrote:
On 05.08.2019 11:40, Julien Grall wrote:
Hi,

On 09/04/2019 12:42, Jan Beulich wrote:
On 09.04.19 at 13:26, <julien.grall@xxxxxxx> wrote:
On 03/04/2019 14:04, Jan Beulich wrote:
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.

I haven't replicated the ` because I have no idea what they are used for. I
would appreciate if you provide pointer how to use them.

Well, I can only point you at the history of things, e.g.
262e118a37 "docs/html/: Annotations for two hypercalls".

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?

While switching from cmd from signed to unsigned should be ok. This would
introduce a different behavior of for count.

Since this removes an error condition, I think this is an okay change
to happen, without ...

Are we happy with that, or shall we add a check ((int)count) > 0?

... any such extra check. This also isn't going to introduce any new
real risk of a long running operation or some such - if 2Gb of input
data are fine, I can't see why 4Gb wouldn't be, too.

Actually, it will not be fine at least for the read case. The return value is a 
32-bit value (on AArch32 and if you want to keep compat between 64-bit and 
32-bit). A negative value indicates an error. As we return the number of 
characters read, it means we can only handle 2GB.

Hmm, valid point.

So I would rather limit the buffer to 2GB for everyone.

Probably fair enough then (if explicitly stated). Personally I would
nevertheless allow sizes up to 4Gb-4kb, but I can see why this may
not be liked by everyone. I'd like to point out though that the
PTR_ERR() machinery we've inherited from Linux utilizes exactly that.

Well, that's a console buffer. The chance you have write/read more than 2GB in a row is very unlikely.

So it feels to me that exposing PTR_ERR(...) like interface is not worth it.

Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.