Re: [PATCH 3/4] tools/libxc: Avoid generating inappropriate zero-length records

[Ian Jackson <ian.jackson@eu.citrix.com>]

David Vrabel writes ("Re: [Xen-devel] [PATCH 3/4] tools/libxc: Avoid generating inappropriate zero-length records"):
> On 21/07/16 18:17, Andrew Cooper wrote:
> > It was never intended for records such as these to be sent with zero content.
> 
> As the original author of the specification I'm perhaps best placed to
> say what the original intention is.

I think this discussion of `the intent' is not particularly helpful,
when the authors of the spec document, and of the code, disagree.  It
is in any case not necessary to decide what `the original intent' is
or was.  Accordingly, can all participants please stop referring to
`the intent' in this way.  (It is of course fine to write `_my_
intent'.)

What is necessary is to decide what should be done now.

I am going to quote liberally from the rest of David's mail but adjust
some of the wording to try to make it something I can agree with:

  For records such as HVM_PARAMS which consist of a set of N items,
  David's intention in the spec, was to most definitely send a record
  with 0 items.

  For records that fetch an opaque blob from the hypervisor, again,
  David's intention was to send this blob as-is with no sort of
  processing or other checking. i.e., if the hypervisor gives us a
  zero-length blob we sent that as-is.

  This makes all the streams look the same with all the same records,
  regardless of what hardware platform it was run on.  Including
  zero-length/count records also makes diagnosing problems easier -- the
  empty record is visible in the stream instead of having to remember that
  sometimes these records are deliberately omitted.

  As such, IMO this series should be limited to making the restore
  side handle the zero count sets or zero length blobs if it does not
  do so already.

  The specification should be clarified to note that some records may have
  zero-length blobs or contain zero items.

I reviewed the spec in detail at the time and I agree with David's
point of view as I have rephrased (hopefully without annoying David)
above.

I see no reason why zero-content-length records should be treated as
any kind of special case.

Is the ultimate bug that we are tripping over here simply that the
code calls malloc(0) and then bails if the libc produces NULL (as it
is entitled to do) ?

Thanks,
Ian.

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