WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Gerd Hoffmann <kraxel@xxxxxxx>
Subject: Re: [Xen-devel] 32-on-64: pvfb issue
From: Markus Armbruster <armbru@xxxxxxxxxx>
Date: Wed, 24 Jan 2007 15:24:10 +0100
Cc: Xen devel list <xen-devel@xxxxxxxxxxxxxxxxxxx>, Keir Fraser <keir@xxxxxxxxxxxxx>
Delivery-date: Wed, 24 Jan 2007 06:23:50 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <45B75343.1090409@xxxxxxx> (Gerd Hoffmann's message of "Wed, 24 Jan 2007 13:38:27 +0100")
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <C1DA7F58.801A%keir@xxxxxxxxxxxxx> <45B60548.4060003@xxxxxxx> <45B741B7.1080306@xxxxxxx> <87sle0obnw.fsf@xxxxxxxxxxxxxxxxx> <45B75343.1090409@xxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Gnus/5.1008 (Gnus v5.10.8) Emacs/21.4 (gnu/linux)
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