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
Unclean!
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
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel