From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id B2A8268B0 for ; Fri, 8 Aug 2014 07:49:13 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP; 07 Aug 2014 22:51:48 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.01,823,1400050800"; d="scan'208";a="581810361" Received: from fmsmsx103.amr.corp.intel.com ([10.19.9.34]) by fmsmga002.fm.intel.com with ESMTP; 07 Aug 2014 22:51:47 -0700 Received: from fmsmsx102.amr.corp.intel.com (10.18.124.200) by FMSMSX103.amr.corp.intel.com (10.19.9.34) with Microsoft SMTP Server (TLS) id 14.3.123.3; Thu, 7 Aug 2014 22:51:47 -0700 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by FMSMSX102.amr.corp.intel.com (10.18.124.200) with Microsoft SMTP Server (TLS) id 14.3.123.3; Thu, 7 Aug 2014 22:51:47 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.252]) by shsmsx102.ccr.corp.intel.com ([169.254.2.172]) with mapi id 14.03.0195.001; Fri, 8 Aug 2014 13:51:45 +0800 From: "Xie, Huawei" To: Stephen Hemminger Thread-Topic: [dpdk-dev] [PATCH 1/2] lib/librte_vhost: vhost library support to facilitate integration with vswitch. Thread-Index: AQHPsmlGnwUtxN6tnEGVzBFuCq/OEZvGDKBw Date: Fri, 8 Aug 2014 05:51:44 +0000 Message-ID: References: <1405661946-12534-1-git-send-email-huawei.xie@intel.com> <1405661946-12534-2-git-send-email-huawei.xie@intel.com> <20140807105849.62a80fc1@haswell.linuxnetplumber.net> In-Reply-To: <20140807105849.62a80fc1@haswell.linuxnetplumber.net> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Aug 2014 05:49:14 -0000 > -----Original Message----- > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Friday, August 08, 2014 1:59 AM > To: Xie, Huawei > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH 1/2] lib/librte_vhost: vhost library suppo= rt to > facilitate integration with vswitch. >=20 > On Fri, 18 Jul 2014 13:39:05 +0800 > Huawei Xie wrote: >=20 > > Signed-off-by: Huawei Xie > > Acked-by: Konstantin Ananyev > > Acked-by: Thomos Long >=20 > This looks good, but there are some style convention issues: >=20 > 1. Please don't use really long lines. About 100 or 120 characters is max= imum > reasonable length in an editor >=20 I plan to fix the length issues in subsequent patch. There are plenty of th= em, which are inherited from old vhost code.=20 > 2. Don't put space here in function decl. >=20 > 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= *); > ^ >=20 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 V= 3 patch. > 3. Use BSD and kernel style brace > Not: >=20 > +void > +put_files_struct (struct files_struct *files) > +{ > + if (atomic_dec_and_test (&files->count)) > + { > + BUG (); > + } > +} >=20 > Instead: > +void > +put_files_struct (struct files_struct *files) > +{ > + if (atomic_dec_and_test (&files->count)) { > + BUG (); > + } > +} >=20 > 4. All functions that are not used in other files should be marked static= . > For example put_files_struct >=20 > 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; > + >=20 > 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 w= arning in v3 patch.