[0/3] Rework CTF event description storage

  Following recent increase of an internal array that was limiting CTF event
descriptions, I had a second look at the code.
All of this is slow path, so I see no reason in keeping this limitation
and we can go with dynamic allocations.

While at it, I tweaked the metadata file output.

I consider this -rc2 material.
  

On Fri, Oct 23, 2020 at 10:01 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> Following recent increase of an internal array that was limiting CTF event
> descriptions, I had a second look at the code.
> All of this is slow path, so I see no reason in keeping this limitation
> and we can go with dynamic allocations.
>
> While at it, I tweaked the metadata file output.
>
> I consider this -rc2 material.

Comments?
Thanks.
  

On Wed, Oct 28, 2020 at 1:13 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Fri, Oct 23, 2020 at 10:01 AM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > Following recent increase of an internal array that was limiting CTF event
> > descriptions, I had a second look at the code.
> > All of this is slow path, so I see no reason in keeping this limitation
> > and we can go with dynamic allocations.
> >
> > While at it, I tweaked the metadata file output.
> >
> > I consider this -rc2 material.
>
> Comments?
> Thanks.

The Generated metadata has issues. Please check

Reproducer:

echo "trace_autotest" |  ./build/app/test/dpdk-test  -c 0x3 --trace=.*
--no-huge --trace=.*


[main][dpdk.org] $ babeltrace
/home/jerin/dpdk-traces/rte-2020-10-28-PM-02-02-37/
[error] at line 1303: invalid character '0x11'
[error] at line 1303: token "": syntax error, unexpected ERROR
[error] Error creating AST
[warning] Unable to open trace metadata for path
"/home/jerin/dpdk-traces/rte-2020-10-28-PM-02-02-37".
[warning] [Context] Cannot open_trace of format ctf at path
/home/jerin/dpdk-traces/rte-2020-10-28-PM-02-02-37.
[warning] [Context] cannot open trace
"/home/jerin/dpdk-traces/rte-2020-10-28-PM-02-02-37" from
/home/jerin/dpdk-traces/rte-2020-10-28-PM-02-02-37/ for reading.
[error] Cannot open any trace for reading.
[error] opening trace
"/home/jerin/dpdk-traces/rte-2020-10-28-PM-02-02-37/" for reading.
[error] none of the specified trace paths could be opened.



>
> --
> David Marchand
>
  

On Wed, Oct 28, 2020 at 9:53 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>
> On Wed, Oct 28, 2020 at 1:13 AM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > On Fri, Oct 23, 2020 at 10:01 AM David Marchand
> > <david.marchand@redhat.com> wrote:
> > >
> > > Following recent increase of an internal array that was limiting CTF event
> > > descriptions, I had a second look at the code.
> > > All of this is slow path, so I see no reason in keeping this limitation
> > > and we can go with dynamic allocations.
> > >
> > > While at it, I tweaked the metadata file output.
> > >
> > > I consider this -rc2 material.
> >
> > Comments?
> > Thanks.
>
> The Generated metadata has issues. Please check
>
> Reproducer:
>
> echo "trace_autotest" |  ./build/app/test/dpdk-test  -c 0x3 --trace=.*
> --no-huge --trace=.*

Err, indeed, thanks for catching.
I did some diff on metadata files, but did not notice this trailing character.

This is an issue with the metadata string manipulations, that appears
with the last patch...
I ended up rewriting most of _ctf.c (removing intermediate buffers
allocations) and it works but I'll see if I can pinpoint the issue.
  

On Wed, Oct 28, 2020 at 2:09 PM David Marchand
<david.marchand@redhat.com> wrote:
> > echo "trace_autotest" |  ./build/app/test/dpdk-test  -c 0x3 --trace=.*
> > --no-huge --trace=.*
>
> Err, indeed, thanks for catching.
> I did some diff on metadata files, but did not notice this trailing character.
>
> This is an issue with the metadata string manipulations, that appears
> with the last patch...
> I ended up rewriting most of _ctf.c (removing intermediate buffers
> allocations) and it works but I'll see if I can pinpoint the issue.

The problem is in the current HEAD, but it is revealed by my series,
maybe because the dynamicity around allocations changed.
I see no check on the length of trace->ctf_meta when writing to the
metadata file.
Did I miss something?


int
rte_trace_metadata_dump(FILE *f)
{
...
        rc = fprintf(f, "%s", ctf_meta);
...
}


Breakpoint 1, trace_mkdir () at
../lib/librte_eal/common/eal_common_trace_utils.c:317
317    {
(gdb) p strlen(trace_obj_get()->ctf_meta)
$2 = 21865
(gdb) p trace_obj_get()->ctf_meta[21865]
$3 = 0 '\000'
(gdb) set trace_obj_get()->ctf_meta[21865] = 'A'
...

$ babeltrace $(ls -1rtd $HOME/dpdk-traces/* |tail -1)
[error] at line 1008: token "A": syntax error, unexpected IDENTIFIER
[error] Error creating AST
...


This fixes it:
@@ -37,11 +37,12 @@ meta_copy(char **meta, int *offset, char *str, int rc)
        if (rc < 0)
                return rc;

-       ptr = realloc(ptr, count + rc);
+       ptr = realloc(ptr, count + rc + 1);
        if (ptr == NULL)
                goto free_str;

        memcpy(RTE_PTR_ADD(ptr, count), str, rc);
+       ptr[count + rc] = '\0';
        count += rc;
        free(str);
  

On Wed, Oct 28, 2020 at 4:17 PM David Marchand
<david.marchand@redhat.com> wrote:
> This fixes it:
> @@ -37,11 +37,12 @@ meta_copy(char **meta, int *offset, char *str, int rc)
>         if (rc < 0)
>                 return rc;
>
> -       ptr = realloc(ptr, count + rc);
> +       ptr = realloc(ptr, count + rc + 1);
>         if (ptr == NULL)
>                 goto free_str;
>
>         memcpy(RTE_PTR_ADD(ptr, count), str, rc);
> +       ptr[count + rc] = '\0';
>         count += rc;
>         free(str);
>

The other alternative is to prefer libc string formatting functions
rather than plain memory alloc + copy + manual null termination:
https://github.com/david-marchand/dpdk/commit/traces