|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 3/4] xen/public: Document HYPERCALL_console_io()
On 05.08.2019 12:17, Julien Grall wrote: > 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. Sure, hence me having said "personally I would ..." - I don't expect you to go that route. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |