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

Re: [Xen-devel] 32-on-64: pvfb issue

Gerd Hoffmann <kraxel@xxxxxxx> writes:

>   Hi,
>>> @@ -560,10 +560,10 @@ int xenfb_attach_dom(struct xenfb *xenfb
>>>     if (xenfb_wait_for_frontend_initialised(&xenfb->kbd) < 0)
>>>             goto error;
>>> -   if (xenfb_bind(&xenfb->fb) < 0)
>>> -           goto error;
>>>     if (xenfb_bind(&xenfb->kbd) < 0)
>>>             goto error;
>>> +   if (xenfb_bind(&xenfb->fb) < 0)
>>> +           goto error;
>>>     if (xenfb_xs_scanf1(xsh, xenfb->fb.otherend, "feature-update",
>>>                         "%d", &val) < 0)
>> Why is this patch hunk necessary?
> Oh, forgot to mention that, sorry.  Only vfb has a "protocol" node, vkbd
> hasn't.  So binding fb last makes sure we have the protocol filled
> correctly in the struct.
> cheers,
>   Gerd


You put protocol[] into xenfb_private, which means it's shared between
vfb and vkbd.  That's defensible.  However, you really shouldn't read
it in xenfb_bind() then.  That reads it both from vfb (where it is
valid) and vkbd (where it is currently undefined), and the one read
last wins.

Reading it next to reading feature-update would be much cleaner.

Alternatively, put protocol[] into xenfb_device.

If you insist on keeping it the way it is, you really need a comment.
But cleaning it up should be less work than explaining it.

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.