From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from emea01-db3-obe.outbound.protection.outlook.com (mail-db3on0082.outbound.protection.outlook.com [157.55.234.82]) by dpdk.org (Postfix) with ESMTP id 0A8312C5B for ; Tue, 14 Jun 2016 06:31:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=FC9hWBfbWI9LlOdhQIWdGF6YIvHYhKGviHH9ic92xHk=; b=x9MuGJL0o0sKm8jt05OOghMdydgkPQZ4g24PjjerT4Ox6PU1Qud9jlMfM3wvG5y26b0REPQziUJj3jvDSwAO4GoR2rFSBw5qOb/MvXoDssqLj5/CZpttYdWQ15gyqW8JyToB7J4Kfrj1KAaFAJmXiO95ZPYj6NB3fLbDUOtb0Xc= Received: from DB5PR0401MB2054.eurprd04.prod.outlook.com (10.166.11.137) by DB5PR0401MB2054.eurprd04.prod.outlook.com (10.166.11.137) with Microsoft SMTP Server (TLS) id 15.1.517.8; Tue, 14 Jun 2016 04:31:10 +0000 Received: from DB5PR0401MB2054.eurprd04.prod.outlook.com ([10.166.11.137]) by DB5PR0401MB2054.eurprd04.prod.outlook.com ([10.166.11.137]) with mapi id 15.01.0517.011; Tue, 14 Jun 2016 04:31:10 +0000 From: Shreyansh Jain To: Jan Viktorin CC: "dev@dpdk.org" , David Marchand , Thomas Monjalon , Bruce Richardson , Declan Doherty , "jianbo.liu@linaro.org" , "jerin.jacob@caviumnetworks.com" , "Keith Wiles" , Stephen Hemminger Thread-Topic: [dpdk-dev] [PATCH v1 02/28] eal: extract function eal_parse_sysfs_valuef Thread-Index: AQHRxYAFwsGnVKsKjkWuKvqF/+arRJ/oXhTw Date: Tue, 14 Jun 2016 04:30:57 +0000 Deferred-Delivery: Tue, 14 Jun 2016 04:30:02 +0000 Message-ID: References: <20160613162431.714e56ae@pcviktorin.fit.vutbr.cz> In-Reply-To: <20160613162431.714e56ae@pcviktorin.fit.vutbr.cz> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=shreyansh.jain@nxp.com; x-originating-ip: [192.88.169.1] x-ms-office365-filtering-correlation-id: cb88648e-1bbd-4b36-69c8-08d3940cb5e3 x-microsoft-exchange-diagnostics: 1; DB5PR0401MB2054; 6:6Lus9OxN7Fpu3gIkDYklgeTkkyYTT1AWOmw5aoGVbhcOdVo0nFFpDUjOiwWeN06IAvfe48Ln1E67J4nc/oC1IJ3/Hhpizb+7I4+jXx44GvQOpRlZitNazpg+xV1FuoxP+g6RUsBRLp0MXPBa5lsSVZPhZqiLhr8b53XxmQ2UOwJxsx6hjI9NIqmea+1Jw4hNgHIwV5SiduTNY1I6WmZ3jqzwIXaQIXyh1YEsP12o04QS4x2pz2ggZjQt4ChQ3QQy6Ah3pkCeW/mQZyVGc756hdEdHZ/NmTkLMQYSJaytPySEKxw++JNvwX00TUjpgEwz; 5:Gb8H5Vdd6+eXhyZGz+McEAV8zHatiJyJahRAYcpP4ms+hHk3des2pcTaZkrLYsAUXt+H3zNmpDg7gyuy1sV//vV8CNTgZlcrsbSjMZRQfpI2CpnNOdJX3VB1pX+xuU7Kxq/0vhCcGIdeATHZ0rytoA==; 24:KAltilYYnFFzpGsJHG0vUCG1zMM0KyXwYzB017hvwiGqfh3CpodWU8mi47Ok3wI39nH6OhTp5xBBXZqHsPILlzwYTn14WHCrn6oocOx7lP8=; 7:owznH1yTCdXe9APj0boFuvLAI9n2DCgedT6KtbZO8kJA+E3qMGgKnAgufTaFH1qZIM/MSBNJpw4B8aFjvtM4UXxVhhWOFW6K65m0Yxzmbvv/C0G2I9m0YAObQ/aZOHn1dl5e2kbAsrYfty+h4IKDhrkdQzbJ6CPkBA/fHJserqY9v+xC8KHL6C6TiyROoQrQRYNwfJ/wXdBSP5J7v9j634OiU5bVcIYsmFZkB6E17dY= x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DB5PR0401MB2054; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(185117386973197)(228905959029699); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(601004)(2401047)(5005006)(8121501046)(10201501046)(3002001)(6055026); SRVR:DB5PR0401MB2054; BCL:0; PCL:0; RULEID:; SRVR:DB5PR0401MB2054; x-forefront-prvs: 09730BD177 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(7916002)(377454003)(199003)(24454002)(13464003)(189002)(81156014)(81166006)(8676002)(8936002)(122556002)(5002640100001)(2950100001)(106356001)(66066001)(2900100001)(87936001)(54356999)(106116001)(5003600100002)(101416001)(9686002)(5008740100001)(2906002)(105586002)(33656002)(76576001)(110136002)(19580395003)(19580405001)(68736007)(4326007)(76176999)(50986999)(97736004)(5004730100002)(189998001)(6116002)(86362001)(77096005)(586003)(11100500001)(3660700001)(10400500002)(92566002)(74316001)(3280700002)(3846002)(102836003); DIR:OUT; SFP:1101; SCL:1; SRVR:DB5PR0401MB2054; H:DB5PR0401MB2054.eurprd04.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; CAT:NONE; LANG:en; CAT:NONE; received-spf: None (protection.outlook.com: nxp.com does not designate permitted sender hosts) spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Jun 2016 04:31:09.9328 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB5PR0401MB2054 Subject: Re: [dpdk-dev] [PATCH v1 02/28] eal: extract function eal_parse_sysfs_valuef 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: Tue, 14 Jun 2016 04:31:12 -0000 Hi Jan, > -----Original Message----- > From: Jan Viktorin [mailto:viktorin@rehivetech.com] > Sent: Monday, June 13, 2016 7:55 PM > To: Shreyansh Jain > Cc: dev@dpdk.org; David Marchand ; Thomas Monja= lon > ; Bruce Richardson ; > Declan Doherty ; jianbo.liu@linaro.org; > jerin.jacob@caviumnetworks.com; Keith Wiles ; Step= hen > Hemminger > Subject: Re: [dpdk-dev] [PATCH v1 02/28] eal: extract function > eal_parse_sysfs_valuef >=20 > On Mon, 13 Jun 2016 14:18:40 +0000 > Shreyansh Jain wrote: >=20 > > Hi Jan, > > > > > -----Original Message----- > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jan Viktorin > > > Sent: Friday, May 06, 2016 7:18 PM > > > To: dev@dpdk.org > > > Cc: Jan Viktorin ; David Marchand > > > ; Thomas Monjalon ; > > > Bruce Richardson ; Declan Doherty > > > ; jianbo.liu@linaro.org; > > > jerin.jacob@caviumnetworks.com; Keith Wiles ; > Stephen > > > Hemminger > > > Subject: [dpdk-dev] [PATCH v1 02/28] eal: extract function > > > eal_parse_sysfs_valuef > > > > > > The eal_parse_sysfs_value function accepts a filename however, such > interface > > > introduces race-conditions to the code. Introduce the variant of this > > > function > > > that accepts an already opened file instead of a filename. > > > > > > Signed-off-by: Jan Viktorin > > > --- > > > lib/librte_eal/common/eal_filesystem.h | 5 +++++ > > > lib/librte_eal/linuxapp/eal/eal.c | 36 +++++++++++++++++++++++-= ---- > ---- > > > -- > > > 2 files changed, 30 insertions(+), 11 deletions(-) > > > > > > diff --git a/lib/librte_eal/common/eal_filesystem.h > > > b/lib/librte_eal/common/eal_filesystem.h > > > index fdb4a70..7875454 100644 > > > --- a/lib/librte_eal/common/eal_filesystem.h > > > +++ b/lib/librte_eal/common/eal_filesystem.h > > > @@ -43,6 +43,7 @@ > > > /** Path of rte config file. */ > > > #define RUNTIME_CONFIG_FMT "%s/.%s_config" > > > > > > +#include > > > #include > > > #include > > > #include > > > @@ -115,4 +116,8 @@ eal_get_hugefile_temp_path(char *buffer, size_t > buflen, > > > const char *hugedir, int > > > * Used to read information from files on /sys */ > > > int eal_parse_sysfs_value(const char *filename, unsigned long *val); > > > > > > +/** Function to read a single numeric value from a file on the > filesystem. > > > + * Used to read information from files on /sys */ > > > +int eal_parse_sysfs_valuef(FILE *f, unsigned long *val); > > > + > > > #endif /* EAL_FILESYSTEM_H */ > > > diff --git a/lib/librte_eal/linuxapp/eal/eal.c > > > b/lib/librte_eal/linuxapp/eal/eal.c > > > index 4b28197..e8fce6b 100644 > > > --- a/lib/librte_eal/linuxapp/eal/eal.c > > > +++ b/lib/librte_eal/linuxapp/eal/eal.c > > > @@ -126,13 +126,30 @@ rte_eal_get_configuration(void) > > > return &rte_config; > > > } > > > > > > +int > > > +eal_parse_sysfs_valuef(FILE *f, unsigned long *val) >=20 > Hi Shreyansh, >=20 > > > > Trivial Comment: Maybe it is just me, but this function name is too clo= se > to its caller 'eal_parse_sysfs_value'. Probably, the name of the caller c= an > be changed to 'eal_parse_sysfs' because anyways value parsing is being do= ne > in this ('eal_parse_sysfs_valuef()) function now. And, of course, droppin= g > the '..f' in this name. >=20 > I don't like the idea of renaming the orignal function eal_parse_sysfs_va= lue. > The function > name is not related to its actual body but to its semantics. And the > semantics are still > the same. This would introduce many other unneeded changes... >=20 Agree. I overlooked that. > > > > I almost skipped the '..f' in the name and wondered how two functions > having same name exist :D >=20 > I agree that a better name would be nice here. This convention was based = on > the libc naming > (fopen, fclose) but the "f" letter could not be at the beginning. >=20 > What about one of those? >=20 > * eal_parse_sysfs_fd_value > * eal_parse_sysfs_file_value I don't have any better idea than above. Though, I still feel that 'eal_parse_sysfs_value -> eal_parse_sysfs_file_va= lue' would be slightly asymmetrical - but again, this is highly subjective = argument. Or, eal_parse_sysfs_value -> eal_parse_sysfs_value_read() may be... But, eal_parse_sysfs_file_value is still preferred than eal_parse_sysfs_fd_= value, for me. >=20 > Regards > Jan >=20 > [...] - Shreyansh