just following up.
On Wed, Dec 07, 2022 at 08:38:39AM -0800, Tyler Retzlaff wrote:
> On Wed, Dec 07, 2022 at 10:13:39AM +0100, Mattias Rönnblom wrote:
> >
> > To be consistent with function naming scheme, where "ctrl" is the
> > old API, and "control" the new, you could call the start functions
> > something with "ctrl" and "control" as well.
>
> i'll make this change in v3.
>
> >
> > Maybe it's worth mentioning that fact somewhere in the beginning of
> > the file, as a comment (i.e., that "ctrl" denotes the old API).
>
> i'll make this change in v3.
didn't end up doing this, didn't think it was necessary once i looked at
it the commit history and the rename you suggested above should be
satisfactory.
>
> >
> > >+ } u;
> > > void *arg;
> > > int ret;
> >
> > Why is 'ret' needed? (This question is unrelated to your patch.)
>
> i'm not the original author so difficult to answer authoritatively. but
> if i have to speculate i'd say it might be to work around the windows
> pthread_join stub being implemented as a noop. specifically it didn't
> communicate the return value from the start_routine.
>
> the recently added rte_thread_join addresses this (which
> rte_control_thread_create uses) and could remove ret parameter and to
> avoid touching the new function implementation in the future it can not
> use ret now.
>
> i'll make this change in v3.
also did not do this, once looked at i was concerned about inter-mixing
too much semantic change in the implementation of a couple of the
functions to accomplish it. it is best revisited when the old api is
removed.
> > >+ * the error is ignored and a debug message is logged.
> > >+ *
> > >+ * @param thread
> > >+ * Filled with the thread id of the new created thread.
> > >+ * @param name
> > >+ * The name of the control thread (max 16 characters including '\0').
> >
> > Is there a constant for this limit?
>
> i have a series introducing rte_lcore_{set,get}_name api that introduces
> a constant (probably i'll post it today). as of this series there is no
> constant.
change in v3, i forgot we were talking about the thread length limit and
not the lcore name length limit and there was already a preprocessor
definition for the limit. api documentation comment was updated in v3.
thanks!
