> From: Thomas Monjalon [mailto:tho...@monjalon.net] > Sent: Monday, 30 October 2023 11.35 > > 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 > <m...@smartsharesystems.com> wrote: > > > > > > > From: David Marchand [mailto:david.march...@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.
Me too. > > 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. I agree 100 %. And if all current callers do, then I consider it safe to lower the message level here. New callers are expected to follow the pattern of current callers. > > Acked-by: Thomas Monjalon <tho...@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. > I don't feel strongly about my suggested changes, so however you handle it... Acked-by: Morten Brørup <m...@smartsharesystems.com>
