[libvirt] [PATCH 02/10] Provide a simple object for encoding/decoding RPC messages

[Daniel P. Berrange]

On Thu, Jun 23, 2011 at 04:19:37PM -0600, Eric Blake wrote:
> On 06/22/2011 09:33 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.
> > 
> > Expected usage scenarios are:
> > 
> > To send a message
> > 
> >    msg = virNetMessageNew()
> > 
> >    ...fill in msg->header fields..
> >    virNetMessageEncodeHeader(msg)
> >    ...loook at msg->header fields to determine payload filter
> >    virNetMessageEncodePayload(msg, xdrfilter, data)
> >    ...send msg->bufferLength worth of data from buffer
> > 
> > To receive a message
> > 
> >    msg = virNetMessageNew()
> >    ...read VIR_NET_MESSAGE_LEN_MAX of data into buffer
> >    virNetMessageDecodeLength(msg)
> >    ...read msg->bufferLength-msg->bufferOffset of data into buffer
> >    virNetMessageDecodeHeader(msg)
> >    ...look at msg->header fields to determine payload filter
> >    virNetMessageDecodePayload(msg, xdrfilter, data)
> >    ...run payload processor
> 
> 'make check' fails:
> 
> virnetmessagetest-virnetmessagetest.o: In function
> `testMessageHeaderEncode':
> /home/remote/eblake/libvirt/tests/virnetmessagetest.c:57: undefined
> reference to `virNetMessageEncodeHeader'
> 
> But that was easy enough to fix.

Sigh. I have rebased this patch series sooooo many times
now, that I lost some pieces :-( Your suggested change
is what I originally had.

> > +++ b/src/rpc/virnetmessage.h
> > @@ -0,0 +1,82 @@
> > +/*
> 
> > +
> > +#ifndef __VIR_NET_MESSAGE_H__
> > +# define __VIR_NET_MESSAGE_H__
> > +
> > +# include <stdbool.h>
> 
> Is this include still necessary?

No, it should be able to go.

> 
> > +int virtTestDifferenceBin(FILE *stream,
> > +                          const char *expect,
> > +                          const char *actual,
> > +                          size_t length)
> > +{
> > +    size_t start = 0, end = length;
> > +    ssize_t i;
> > +
> > +    if (!virTestGetDebug())
> > +        return 0;
> > +
> > +    if (virTestGetDebug() < 2) {
> > +        /* Skip to first character where they differ */
> > +        for (i = 0 ; i < length ; i++) {
> > +            if (expect[i] != actual[i]) {
> > +                start = i;
> > +                break;
> > +            }
> > +        }
> > +
> > +        /* Work backwards to last character where they differ */
> > +        for (i = (length -1) ; i >= 0 ; i--) {
> > +            if (expect[i] != actual[i]) {
> > +                end = i;
> > +                break;
> > +            }
> > +        }
> > +    }
> > +    /* Round to nearest boundary of 4 */
> > +    start -= (start % 4);
> > +    end += 4 - (end % 4);
> 
> This could make end go beyond length, if the difference was in the last
> byte or two on an unaligned length.  And if expect or actual are also
> unaligned, then this means that reading beyond bounds could segfault (at
> any rate, valgrind will certainly call you on it).

Hmm, interesting edge case.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|