From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 2E2F75952 for ; Thu, 21 Jul 2016 22:50:46 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga103.jf.intel.com with ESMTP; 21 Jul 2016 13:50:44 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,401,1464678000"; d="scan'208";a="737929202" Received: from irsmsx110.ger.corp.intel.com ([163.33.3.25]) by FMSMGA003.fm.intel.com with ESMTP; 21 Jul 2016 13:50:43 -0700 Received: from irsmsx112.ger.corp.intel.com (10.108.20.5) by irsmsx110.ger.corp.intel.com (163.33.3.25) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 21 Jul 2016 21:50:43 +0100 Received: from irsmsx110.ger.corp.intel.com ([169.254.15.113]) by irsmsx112.ger.corp.intel.com ([169.254.1.33]) with mapi id 14.03.0248.002; Thu, 21 Jul 2016 21:50:42 +0100 From: "Jastrzebski, MichalX K" To: Thomas Monjalon CC: "dev@dpdk.org" , "Richardson, Bruce" , "Kobylinski, MichalX" , "Gonzalez Monroy, Sergio" , "david.marchand@6wind.com" Thread-Topic: [dpdk-dev] [PATCH] eal: fix check number of bytes from read function Thread-Index: AQHR4pKMTK4JoI4nYkS4mUPeQR5mjqAi5GoAgAB0yDA= Date: Thu, 21 Jul 2016 20:50:41 +0000 Message-ID: <60ABE07DBB3A454EB7FAD707B4BB158213AB97E7@irsmsx110.ger.corp.intel.com> References: <1469024689-1076-1-git-send-email-michalx.k.jastrzebski@intel.com> <1862165.0hl8WtbyGd@xps13> In-Reply-To: <1862165.0hl8WtbyGd@xps13> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] eal: fix check number of bytes from read function 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: Thu, 21 Jul 2016 20:50:46 -0000 > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Thursday, July 21, 2016 4:36 PM > To: Jastrzebski, MichalX K > Cc: dev@dpdk.org; Richardson, Bruce ; > Kobylinski, MichalX ; Gonzalez Monroy, > Sergio ; david.marchand@6wind.com > Subject: Re: [dpdk-dev] [PATCH] eal: fix check number of bytes from read > function >=20 > Hi, >=20 > 2016-07-20 16:24, Michal Jastrzebski: > > - if (read(fd, &page, sizeof(uint64_t)) < 0) { > > + > > + retval =3D read(fd, &page, sizeof(uint64_t)); > > + if (retval < 0) { > > RTE_LOG(ERR, EAL, "%s(): cannot read /proc/self/pagemap: > %s\n", > > __func__, strerror(errno)); > > close(fd); > > return RTE_BAD_PHYS_ADDR; > > + } else if (retval >=3D 0 && retval < (int)sizeof(uint64_t)) { >=20 Hi Thomas, > I have 4 comments about the above line: That's too much for one line. I should improve next time:) > - the check retval >=3D 0 is not needed because implied by else > - why not checking retval !=3D sizeof(uint64_t) as it is the exact expect= ed > value? Yes, it is better solution, > - (int)sizeof(uint64_t) can be replaced by 8 but it's shorter ;) I didn't want to change all invokes of read() function here.=20 I can use some macro: #define PFN_MASK_SIZE 8 How do You think? > - only 1 space is required between } and else >=20 > > + RTE_LOG(ERR, EAL, "%s(): read %d bytes from > /proc/self/pagemap " > > + "but expected %d: %s\n", > > + __func__, retval, (int)sizeof(uint64_t), > strerror(errno)); >=20 > Are you sure errno is meaningful here? I think it is not. Will send v2. >=20 > > + close(fd); > > + return RTE_BAD_PHYS_ADDR; > > } Thanks for a review Michal.