Tue, Feb 11, 2025 at 03:38:25PM +0100, [email protected] wrote: >On 2/11/25 13:52, Jiri Pirko wrote: >> Tue, Feb 11, 2025 at 01:12:12PM +0100, [email protected] wrote: >> > From: Jiri Pirko <[email protected]> >> > Sent: Monday, February 10, 2025 5:26 PM >> > > Mon, Feb 10, 2025 at 02:56:28PM +0100, [email protected] wrote: >> > > >> > > [...] >> > > >> > > > +enum ixgbe_devlink_version_type { >> > > > + IXGBE_DL_VERSION_FIXED, >> > > > + IXGBE_DL_VERSION_RUNNING, >> > > > +}; >> > > > + >> > > > +static int ixgbe_devlink_info_put(struct devlink_info_req *req, >> > > > + enum ixgbe_devlink_version_type type, >> > > > + const char *key, const char *value) >> > > >> > > I may be missing something, but what's the benefit of having this helper >> > > instead of calling directly devlink_info_version_*_put()? >> > >> > ixgbe devlink .info_get() supports various adapters across ixgbe portfolio >> > which >> > have various sets of version types - some version types are not applicable >> > for some of the adapters - so we want just to check if it's *not empty.* >> > >> > If so then we don't want to create such entry at all so avoid calling >> > devlink_info_version_*_put() in this case. >> > Putting value check prior each calling of devlink_info_version_*_put() >> > would provide quite a code redundancy and would look not so good imho. >> > >> > Me and Przemek are not fully convinced by adding such additional >> > layer of abstraction but we defineltly need this value check to not >> > print empty type or get error and return from the function. >> > >> > Another solution would be to add such check to devlink function. >> >> That sounds fine to me. Someone else may find this handy as well. >Cool! > >perhaps we could also EXPORT devlink_info_version_put(), that would also >help us reduce number of wrappers (also in other intel drivers)
Why not. Make sure you sanitize attr value.
