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] [PATCH 0/4] [Net] Support accelerated network plugin mod

To: Kieran Mansley <kmansley@xxxxxxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 0/4] [Net] Support accelerated network plugin modules
From: Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>
Date: Fri, 15 Jun 2007 12:26:03 +0100
Cc: netdev@xxxxxxxxxxxxxxx, herbert@xxxxxxxxxxxxxxxxxxx
Delivery-date: Fri, 15 Jun 2007 04:21:33 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <1181904378.4121.51.camel@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
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>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcevP/ToM5wSJBszEdyfcQAWy6hiGQ==
Thread-topic: [Xen-devel] [PATCH 0/4] [Net] Support accelerated network plugin modules
User-agent: Microsoft-Entourage/11.3.3.061214
On 15/6/07 11:46, "Kieran Mansley" <kmansley@xxxxxxxxxxxxxx> wrote:

> This is a repost of some earlier patches to the xen-devel mailing list,
> with a number of changes thanks to some useful suggestions from others.

The coding style needs fixing in various ways.

Hard tabs need to be used, no spaces inside brackets, but should include
spaces between if/while/for and bracket, and bracket and brace:
    if (foo) {
Not
    if( foo ){
    if(foo ) {
Or various other possibilities.

No use of the following please:
If (foo) return 1; else return 0;
Is clearer as:
Return foo;

There's a (good) idiom for error handling in Linux:
If (!error) {
    page of code;
} else {
    oh no, error, print and return;
}
Should be changed to:
If (error) {
    handle error, bail if necessary
}
Page of code, no longer indented by error check;

Apart from the various coding style bits, accelerator_probe_vifs_on_load()
walks the vif_states list with no lock held. Is this safe against the list
changing?

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel