Re: [dpdk-dev] [PATCH 1/2] lib/librte_vhost: vhost library support to facilitate integration with vswitch.

[Stephen Hemminger <stephen@networkplumber.org>]

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