[dpdk-dev] [PATCH v1 02/28] eal: extract function eal_parse_sysfs_valuef

[Shreyansh Jain]

Sorry, didn't notice this email earlier...
Comments inline

> -----Original Message-----
> From: Jan Viktorin [mailto:viktorin at rehivetech.com]
> Sent: Wednesday, June 15, 2016 3:26 PM
> To: Shreyansh Jain <shreyansh.jain at nxp.com>
> Cc: dev at dpdk.org; David Marchand <david.marchand at 6wind.com>; Thomas Monjalon
> <thomas.monjalon at 6wind.com>; Bruce Richardson <bruce.richardson at intel.com>;
> Declan Doherty <declan.doherty at intel.com>; jianbo.liu at linaro.org;
> jerin.jacob at caviumnetworks.com; Keith Wiles <keith.wiles at intel.com>; Stephen
> Hemminger <stephen at networkplumber.org>
> Subject: Re: [dpdk-dev] [PATCH v1 02/28] eal: extract function
> eal_parse_sysfs_valuef
> 
> On Tue, 14 Jun 2016 04:30:57 +0000
> Shreyansh Jain <shreyansh.jain at nxp.com> wrote:
> 
> > Hi Jan,
> >

[...]


> > > >
> > > > I almost skipped the '..f' in the name and wondered how two functions
> > > having same name exist :D
> > >
> > > 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.
> > >
> > > What about one of those?
> > >
> > > * 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_value' would be slightly asymmetrical - but again, this
> is highly subjective argument.
> 
> I don't see any asymmetry here. The functions equal, just the new one accepts
> a file pointer instead of a path
> and we don't have function name overloading in C.

Asymmetrical because cascading function names maybe additive for easy reading/recall.

'eal_parse_sysfs_value ==> eal_parse_sysfs_value_<XX> ==> eal_parse_sysfs_value_<XX>_<YY>'

Obviously, this is not a rule - it just makes reading and recalling of cascade easier.
As for:

eal_parse_sysfs_value => eal_parse_sysfs_file_value

inserts an identifier between a name, making it (slightly) difficult to correlate.

Again, as I mentioned earlier, this is subjective argument and matter of (personal!) choice.

> 
> >
> > Or, eal_parse_sysfs_value -> eal_parse_sysfs_value_read() may be...
> 
> I think, I'll go with eal_parse_sysfs_file_value for v2. Ideally, it should
> be
> eal_parse_sysfs_path_value and eal_parse_sysfs_file_value. Thus, this looks
> like
> a good way.
> 
> >
> > But, eal_parse_sysfs_file_value is still preferred than
> eal_parse_sysfs_fd_value, for me.
> 
> Agree.
> 
[...]

-
Shreyansh