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

[Jan Viktorin <viktorin@rehivetech.com>]

Hello Shreyansh,

On Thu, 16 Jun 2016 11:47:29 +0000
Shreyansh Jain <shreyansh.jain@nxp.com> wrote:

> Sorry, didn't notice this email earlier...
> Comments inline
> 
> > -----Original Message-----
> > From: Jan Viktorin [mailto:viktorin@rehivetech.com]
> > Sent: Wednesday, June 15, 2016 3:26 PM
> > To: Shreyansh Jain <shreyansh.jain@nxp.com>
> > Cc: dev@dpdk.org; David Marchand <david.marchand@6wind.com>; Thomas Monjalon
> > <thomas.monjalon@6wind.com>; Bruce Richardson <bruce.richardson@intel.com>;
> > Declan Doherty <declan.doherty@intel.com>; jianbo.liu@linaro.org;
> > jerin.jacob@caviumnetworks.com; Keith Wiles <keith.wiles@intel.com>; Stephen
> > Hemminger <stephen@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@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.
> >   
> [...]

I've finally returned to your idea to name it eal_parse_sysfs_value_read.

Thanks.
Jan

> 
> -
> Shreyansh



-- 
   Jan Viktorin                  E-mail: Viktorin@RehiveTech.com
   System Architect              Web:    www.RehiveTech.com
   RehiveTech
   Brno, Czech Republic