> -----Original Message----- > From: Stephen Hemminger [mailto:stephen at networkplumber.org] > Sent: Friday, August 08, 2014 1:59 AM > To: Xie, Huawei > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 1/2] lib/librte_vhost: vhost library support to > facilitate integration with vswitch. > > On Fri, 18 Jul 2014 13:39:05 +0800 > Huawei Xie <huawei.xie at intel.com> wrote: > > > Signed-off-by: Huawei Xie <huawei.xie at intel.com> > > Acked-by: Konstantin Ananyev <konstantin.ananyev at intel.com> > > Acked-by: Thomos Long <thomas.long at 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 > I plan to fix the length issues in subsequent patch. There are plenty of them, which are inherited from old vhost code. > 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 > *); > ^ > In this patch, I only run checkpatch.pl against the source file I modified rather than the patch itself, so missed these checking. I had sent the V3 patch in which those kind of issues are fixed. Please refer to the latest V3 patch. > 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
Thanks. As stated above, have fixed all the style issues except 80 length warning in v3 patch.