[dpdk-dev] [EXTERNAL] Re: [PATCH v6 02/10] eal: add thread attributes

[Tyler Retzlaff]

-----Original Message-----
From: Thomas Monjalon <thomas at monjalon.net> 
Sent: Thursday, April 29, 2021 12:48 AM


29/04/2021 02:50, Dmitry Kozlyuk:
> > 2021-04-02 18:39 (UTC-0700), Narcisa Ana Maria Vasile:
> > > +int
> > > +rte_thread_attr_init(rte_thread_attr_t *attr) {
> > > +	if (attr == NULL) {
> > > +		RTE_LOG(DEBUG, EAL,
> > > +		"Unable to init thread attributes, invalid parameter\n");
> > > +		return EINVAL;
> > > +	}
> > 
> > This message doesn't add value for debugging: caller already knows 
> > that attribute initialization failed (that's what function attempts to 
> > do) and that the parameter is invalid (EINVAL).
> > I'd remove it (same applies below).
> > If you find it useful to keep, an extra indent missing (also more below).

> Recently in ethdev we added more messages like this for NULL parameters.
> I agree it is not a lot useful but I understand that lazy developers may like it ;)

Shouldn't this specific case be an assert?  Unless we are trying to maintain compatibility with existing badly designed semantics.
The whole calling pattern is non-sensible, the caller passes an NULL parameter to a function where the input contract is non-NULL and then proceeds to handle the error by doing what that could possibly be useful exactly?