On Tue, Oct 18, 2022 at 02:14:37PM +0100, Bruce Richardson wrote:
> On Thu, Oct 13, 2022 at 09:49:28AM +0200, David Marchand wrote:
> > Register telemetry commands to list and configure trace points and later
> > save traces for a running DPDK application.
> >
> > Note: trace point names contain a '.', so the list of valid characters
> > used in telemetry commands and dictionary keys has been extended.
> >
<snip>
> > diff --git a/lib/telemetry/telemetry_data.c b/lib/telemetry/telemetry_data.c
> > index 34366ecee3..5b319c18fb 100644
> > --- a/lib/telemetry/telemetry_data.c
> > +++ b/lib/telemetry/telemetry_data.c
> > @@ -106,6 +106,7 @@ valid_name(const char *name)
> > ['a' ... 'z'] = 1,
> > ['_'] = 1,
> > ['/'] = 1,
> > + ['.'] = 1,
> > };
> > while (*name != '\0') {
> > if ((size_t)*name >= RTE_DIM(allowed) || allowed[(int)*name] ==
> > 0)
>
> I don't see an issue with allowing "." characters in dictionary names, so
> for this part:
>
> Acked-by: Bruce Richardson <[email protected]>
By way of additional minor follow-up:
* I think the addition of "." to the list of allowed characters should be a
separate patch, rather than just added as part of this larger patch. If
doing a separate commit to add boolean type, this could be a change in that
set.
* There are a couple of API doxygen comments for the dictionary functions,
rte_tel_add_dict_*, where the allowed characters in the name are called
out. You probably should add "." to those comments too.
Regards,
/Bruce
