From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <stephen@networkplumber.org>
Received: from mail-pa0-f54.google.com (mail-pa0-f54.google.com
 [209.85.220.54]) by dpdk.org (Postfix) with ESMTP id AA47918F
 for <dev@dpdk.org>; Thu,  7 Aug 2014 19:56:24 +0200 (CEST)
Received: by mail-pa0-f54.google.com with SMTP id fa1so5850785pad.27
 for <dev@dpdk.org>; Thu, 07 Aug 2014 10:58:57 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20130820;
 h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to
 :references:mime-version:content-type:content-transfer-encoding;
 bh=wUxFkJOsDPvUBLkZZsCTZz2vk5RIUhdAI68pLhOWmOM=;
 b=QKZQLI2++ew3tAffanpZt7cg2ma1LdOMoHCxmVtmdKOhSS+jhs1uaZdsXWXvx0dApq
 MBnWjI5YXiVCUki6i5pPktsyHIC37LMOql2HpNTJRkku1HDNWZS9Tt/dvX6QXSs3Pf8W
 n2Oc2aLmlrULWBU95ytcB8M57lK7aX3Pq4/r2xufbVpsTkg86V7SYc3e6TyV/VHwfFQO
 bCNgjg3WDancv2AFXuhhwgdjUlxQozz5rpmqZA0OgM9WpEKa8mLUvf5zD3NhjCkA2Qi3
 KslRM4TmYKc/MCZIyL3x2vY5yR7Xem8Yug3Mrj1YbfJrw46Gft0VmLFu4/9uc06hEpzI
 P1qA==
X-Gm-Message-State: ALoCoQlwVX2LYy5lA3m2wVZwmrzO8W/ljbGWmJn+4rIRnoIO+0NXom7btVYHhIG3YtzTT7Qg9NjE
X-Received: by 10.68.112.225 with SMTP id it1mr19298765pbb.23.1407434336948;
 Thu, 07 Aug 2014 10:58:56 -0700 (PDT)
Received: from haswell.linuxnetplumber.net
 (static-50-53-65-80.bvtn.or.frontiernet.net. [50.53.65.80])
 by mx.google.com with ESMTPSA id kt2sm489045pbc.83.2014.08.07.10.58.56
 for <multiple recipients>
 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
 Thu, 07 Aug 2014 10:58:56 -0700 (PDT)
Date: Thu, 7 Aug 2014 10:58:49 -0700
From: Stephen Hemminger <stephen@networkplumber.org>
To: Huawei Xie <huawei.xie@intel.com>
Message-ID: <20140807105849.62a80fc1@haswell.linuxnetplumber.net>
In-Reply-To: <1405661946-12534-2-git-send-email-huawei.xie@intel.com>
References: <1405661946-12534-1-git-send-email-huawei.xie@intel.com>
 <1405661946-12534-2-git-send-email-huawei.xie@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 1/2] lib/librte_vhost: vhost library support
 to facilitate integration with vswitch.
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Thu, 07 Aug 2014 17:56:25 -0000

On Fri, 18 Jul 2014 13:39:05 +0800
Huawei Xie <huawei.xie@intel.com> wrote:

> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Acked-by: Thomos Long <thomas.long@intel.com>

This looks good, but there are some style convention issues:

1. Please don't use really long lines. About 100 or 120 characters is maximum
   reasonable length in an editor

2. Don't put space here in function decl.

ERROR: space prohibited after that '*' (ctx:BxW)
#1183: FILE: lib/librte_vhost/vhost-net-cdev.h:102:
+	int (* set_vring_kick)(struct vhost_device_ctx, struct vhost_vring_file *);
 	     ^

3. Use BSD and kernel style brace
Not:

+void
+put_files_struct (struct files_struct *files)
+{
+	if (atomic_dec_and_test (&files->count))
+	{
+		BUG ();
+	}
+}

Instead:
+void
+put_files_struct (struct files_struct *files)
+{
+	if (atomic_dec_and_test (&files->count)) {
+		BUG ();
+	}
+}

4. All functions that are not used in other files should be marked static.
   For example put_files_struct

5. Switch should be indented at same level as case:
Not:
+	switch (ioctl)
+	{
+		case EVENTFD_COPY:
+			if (copy_from_user (&eventfd_copy, argp, sizeof (struct eventfd_copy)))
+				return -EFAULT;
+

Instead:
+	switch (ioctl) {
+	case EVENTFD_COPY:
+		if (copy_from_user (&eventfd_copy, argp, sizeof (struct eventfd_copy)))
+			return -EFAULT