RE: [PATCH 05/11] eal: force prefix for internal threads

["Morten Brørup" <mb@smartsharesystems.com>]

> From: David Marchand [mailto:david.marchand@redhat.com]
> Sent: Thursday, 7 September 2023 10.55
> 
> On Thu, Sep 7, 2023 at 10:53 AM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > On Thu, Sep 7, 2023 at 10:50 AM Morten Brørup <mb@smartsharesystems.com>
> wrote:
> > > > This 10 value in the comment is easy to miss if some change with the
> > > > prefix is done.
> > > > Mentionning RTE_THREAD_INTERNAL_NAME_SIZE is enough.
> > >
> > > I disagree with David's comment to this.
> > >
> > > The function documentation is easier to read if the actual number is also
> mentioned.
> > >
> > > For the best of both worlds, you can add something like this nearby:
> > >
> > > _Static_assert(sizeof(RTE_THREAD_NAME_PREFIX) == sizeof("dpdk-"),
> > >                 "Length of RTE_THREAD_NAME_PREFIX has changed; "
> > >                 "the documentation needs updating.");
> >
> > And how will it catch the comment about 10 characters ?
> 
> I mean you still have to re-read the whole documentation and look for
> some reference somewhere about 10 characters.

The trick is to put the _Static_assert close to where the expectation occurs. That makes it easier to find where changes are necessary.

And the _Static_assert can be added at all the locations where changes would be necessary. (Generally, we should add a lot more _Static_assert to the code where it makes assumptions about e.g. the ordering of fields in a struct, such as the vector optimized code.)

Also, the failure message could be improved to include help about what to look for.

PS: The reference to RTE_THREAD_INTERNAL_NAME_SIZE should remain in the documentation, so perhaps look for "RTE_THREAD_INTERNAL_NAME_SIZE".