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

Re: [libvirt] [PATCH 02/15] Provide a simple object for encoding/decoding RPC messages



On Thu, Dec 16, 2010 at 10:49:16AM -0700, Eric Blake wrote:
> On 12/16/2010 04:21 AM, Daniel P. Berrange wrote:
> > This provides a new struct that contains a buffer for the RPC
> > message header+payload, as well as a decoded copy of the message
> > header. There is an API for applying a XDR encoding & decoding
> > of the message headers and payloads. There are also APIs for
> > maintaining a simple FIFO queue of message instances.
> > 
> 
> > +
> > +/*
> > + * @msg: the outgoing message, whose header to encode
> > + *
> > + * Encodes the length word and header of the  message, setting the
> 
> spacing: s/  / /
> 
> > +
> > +int virNetMessageEncodePayload(virNetMessagePtr msg,
> > +                               xdrproc_t filter,
> > +                               void *data)
> > +{
> > +    XDR xdr;
> > +    unsigned int msglen;
> > +
> > +    /* Serialise header followed by args. */
> 
> This comment is out of place, since the header should already have been
> serialized prior to this function.
> 
> > +
> > +
> > +int virNetMessageDecodePayload(virNetMessagePtr msg,
> > +                               xdrproc_t filter,
> > +                               void *data)
> > +{
> > +    XDR xdr;
> > +
> > +    /* Serialise header followed by args. */
> 
> Likewise.
> 
> > +
> > +int virNetMessageEncodePayloadRaw(virNetMessagePtr msg,
> > +                                  const char *data,
> > +                                  size_t len)
> > +{
> > +    XDR xdr;
> > +    unsigned int msglen;
> > +
> > +    if ((msg->bufferLength - msg->bufferOffset) < len) {
> > +        virNetError(VIR_ERR_RPC,
> > +                    _("Stream data too long to send (%zu bytes needed, %zu bytes available)"),
> > +                    len, (msg->bufferLength - msg->bufferOffset));
> > +        return -1;
> > +    }
> > +
> > +    memcpy(msg->buffer + msg->bufferOffset, data, len);
> > +    msg->bufferOffset += len;
> 
> Should this round up to a multiple of 4, per the rules of XDR encoding?

If this had been raised when we first added the stream
support I'd have said yes, but adding padding would
cause backcompatibility issues now with new vs old client
and server combos. In any case there's nothing in our
RPC code in this stream context which makes use of 4 byte
boundaries more or less efficient than non-aligned data

Regards,
Daniel


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