Re: [PATCH] eal/unix: lower log level for reading files

[Thomas Monjalon <thomas@monjalon.net>]

I would add "sysfs" in the title.

27/10/2023 11:13, David Marchand:
> On Fri, Oct 27, 2023 at 11:00 AM Morten Brørup <mb@smartsharesystems.com> wrote:
> >
> > > From: David Marchand [mailto:david.marchand@redhat.com]
> > > Sent: Friday, 27 October 2023 10.01
> > >
> > > The eal_parse_sysfs_value helper both returns an error code and logs an
> > > error level message when something goes wrong.
> > > On the other hand, internal users of this helper either ignore this
> > > error code (like when trying to find out some numa information from the
> > > Linux sysfs, or discovering some optional feature), or add their own
> > > error
> > > logging when reading the file actually matters.
> > >
> > > Lower this helper log messages to debug level as it provides no useful
> > > information to final DPDK users.
> >
> > Such assumptions seem risky.
> >
> > Please add __attribute__ ((warn_unused_result)) to this function's header, to support the assumption.
> 
> I can add this.

Would be __rte_warn_unused_result

I'm not sure it is required to mandate checking the result.
I'm fine with or without it.

I agree with this patch because not having a sysfs entry is never critical in itself.
If the entry is required in a case, it should be handled by the caller
with a more meaningful message.

Acked-by: Thomas Monjalon <thomas@monjalon.net>


> > Alternatively, add a "bool may_not_exist" parameter to the function to choose the relevant log level.
> 
> If an API update is to be considered, I would rather add some new
> helpers with Windows support.

Please don't change the API for such detail.