From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Hemminger <stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org>
Subject: Re: [PATCH 1/2] lib/librte_vhost: vhost library support
 to facilitate integration with vswitch.
Date: Thu, 7 Aug 2014 10:58:49 -0700
Message-ID: <20140807105849.62a80fc1@haswell.linuxnetplumber.net>
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-VfR2kkLFssw@public.gmane.org
To: Huawei Xie <huawei.xie-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Return-path: <dev-bounces-VfR2kkLFssw@public.gmane.org>
In-Reply-To: <1405661946-12534-2-git-send-email-huawei.xie-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev-VfR2kkLFssw@public.gmane.org>
List-Help: <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=subscribe>
Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org
Sender: "dev" <dev-bounces-VfR2kkLFssw@public.gmane.org>

On Fri, 18 Jul 2014 13:39:05 +0800
Huawei Xie <huawei.xie-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:

> Signed-off-by: Huawei Xie <huawei.xie-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Acked-by: Konstantin Ananyev <konstantin.ananyev-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Acked-by: Thomos Long <thomas.long-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

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