2021-04-29 15:52 (UTC+0000), Tyler Retzlaff:
> -----Original Message-----
> From: Thomas Monjalon <tho...@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?
+1 in this case.
This function is only specified to diagnose ENOMEM. On my Linux machine
pthread_attr_init(NULL) crashes, I guess that would be compatible behavior.
