>
> On Tue, Jan 31, 2017 at 07:45:44AM -0500, Frediano Ziglio wrote:
> > Title should be test-vdagent (with dash).
>
> Thanks, fixed.
>
> >
> > >
> > > This is a revert of 93b4f4050^ and 93b4f4050.
> > > For a SpiceCharDeviceInstance, the base interface must be a
> > > SpiceCharDeviceInterface instance, not a SpiceBaseInterface instance, or
> > > spice-server code will end up reading out of bounds.
> > >
> > > vmc_state/vmc_write/vmc_read implementations also have to be provided.
> > > ---
> > > server/tests/test-vdagent.c | 65
> > > ++++++++++++++++++++++++++++++++++++++++-----
> > > 1 file changed, 59 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/server/tests/test-vdagent.c b/server/tests/test-vdagent.c
> > > index 7f905ad..e06229e 100644
> > > --- a/server/tests/test-vdagent.c
> > > +++ b/server/tests/test-vdagent.c
> > > @@ -45,11 +45,64 @@ static void pinger(SPICE_GNUC_UNUSED void *opaque)
> > > core->timer_start(ping_timer, ping_ms);
> > > }
> > >
> > > -static SpiceBaseInterface base = {
> > > - .type = SPICE_INTERFACE_CHAR_DEVICE,
> > > - .description = "test spice virtual channel char device",
> > > - .major_version = SPICE_INTERFACE_CHAR_DEVICE_MAJOR,
> > > - .minor_version = SPICE_INTERFACE_CHAR_DEVICE_MINOR,
> > > +static int vmc_write(SPICE_GNUC_UNUSED SpiceCharDeviceInstance *sin,
> > > + SPICE_GNUC_UNUSED const uint8_t *buf,
> > > + int len)
> > > +{
> > > + return len;
> > > +}
> > > +
> > > +static int vmc_read(SPICE_GNUC_UNUSED SpiceCharDeviceInstance *sin,
> > > + uint8_t *buf,
> > > + int len)
> > > +{
> > > + static uint8_t c = 0;
> > > + static uint8_t message[2048];
> > > + static unsigned pos = 0;
> > > + static unsigned message_size;
> > > + int ret;
> > > +
> > > + if (pos == 0) {
> > > + VDIChunkHeader *hdr = (VDIChunkHeader *)message;
> > > + VDAgentMessage *msg = (VDAgentMessage *)&hdr[1];
> > > + uint8_t *p = message;
> > > + int size = sizeof(message);
> > > + message_size = size;
> > > + /* fill in message */
> > > + hdr->port = VDP_SERVER_PORT;
> > > + hdr->size = message_size - sizeof(VDIChunkHeader);
> > > + msg->protocol = VD_AGENT_PROTOCOL;
> > > + msg->type = VD_AGENT_END_MESSAGE;
> > > + msg->opaque = 0;
> > > + msg->size = message_size - sizeof(VDIChunkHeader) -
> > > sizeof(VDAgentMessage);
> > > + size -= sizeof(VDIChunkHeader) + sizeof(VDAgentMessage);
> > > + p += sizeof(VDIChunkHeader) + sizeof(VDAgentMessage);
> > > + for (; size; --size, ++p, ++c)
> > > + *p = c;
> > > + }
> > > + ret = MIN(message_size - pos, len);
> > > + memcpy(buf, &message[pos], ret);
> > > + pos += ret;
> > > + if (pos == message_size) {
> > > + pos = 0;
> > > + }
> > > + //printf("vmc_read %d (ret %d)\n", len, ret);
> > > + return ret;
> > > +}
> > > +
> > > +static void vmc_state(SPICE_GNUC_UNUSED SpiceCharDeviceInstance *sin,
> > > + SPICE_GNUC_UNUSED int connected)
> > > +{
> > > +}
> > > +
> > > +static SpiceCharDeviceInterface vmc_interface = {
> > > + .base.type = SPICE_INTERFACE_CHAR_DEVICE,
> > > + .base.description = "test spice virtual channel char device",
> > > + .base.major_version = SPICE_INTERFACE_CHAR_DEVICE_MAJOR,
> > > + .base.minor_version = SPICE_INTERFACE_CHAR_DEVICE_MINOR,
> > > + .state = vmc_state,
> > > + .write = vmc_write,
> > > + .read = vmc_read,
> > > };
> > >
> > > SpiceCharDeviceInstance vmc_instance = {
> > > @@ -63,7 +116,7 @@ int main(void)
> > > core = basic_event_loop_init();
> > > test = test_new(core);
> > >
> > > - vmc_instance.base.sif = &base;
> > > + vmc_instance.base.sif = &vmc_interface.base;
> > > spice_server_add_interface(test->server, &vmc_instance.base);
> > >
> > > ping_timer = core->timer_add(pinger, NULL);
> >
> > How did you discover this problem?
>
> > This test were supposed to be working even without vmc_read, why now
> > is needed? It's not exactly a stub (like "return -1").
> > Maybe NULLs are fine too?
>
> s/SpiceBaseInstance/SpiceCharDeviceInstance/ was found because the test
> was crashing at start. vmc_read implementation is the same as the one
> which was present before 93b4f4050. Arguably, it's not really useful as
> then the test just loops forever on vmc_read, but I'd say that's a
> different issue (and actually I don't really know what the test is
> supposed to be testing /o\ )
> Without a vmc_read implementation, you get a crash when trying to read
> from the device in vdi_port_read_one_msg_from_device(). There used to be
> an assert when one of the vfuncs was missing, which alos caused me to
> add this.
>
> Christophe
>
I don't know what this test is supposed to do but change a crash
into an infinite loop.
Original commit message:
add server/tests/test_vdagent
File comment:
Test vdagent guest to server messages
suggestions of what should be the behaviour?
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel