[PATCH 05/11] eal: force prefix for internal threads
Morten Brørup
mb at smartsharesystems.com
Thu Sep 7 13:10:51 CEST 2023
> From: David Marchand [mailto:david.marchand at redhat.com]
> Sent: Thursday, 7 September 2023 10.55
>
> On Thu, Sep 7, 2023 at 10:53 AM David Marchand
> <david.marchand at redhat.com> wrote:
> >
> > On Thu, Sep 7, 2023 at 10:50 AM Morten Brørup <mb at 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".
More information about the dev
mailing list