[DTrace-devel] [PATCH v2 13/20] Store strings in the string table with varint length prefix

[Eugene Loh]

Reviewed-by: Eugene Loh <eugene.loh at oracle.com>

In
         libdtrace/dt_bpf.c
         libdtrace/dt_cc.c
         libdtrace/dt_dis.c
I see
         +#include <dt_varint.h>
Why?  I think this include is not needed since you encapsulate the vint 
stuff in the new function, which is declared in dt_impl.h.

In
         dt_strtab_index()
         dt_strtab_insert()
is "s = malloc()" a memory leak?  In the case of dt_strtab_insert(), 
freeing s means paying attention to multiple return paths.

Finally...

On 6/3/21 11:17 AM, Kris Van Hees wrote:
> With the introduction of variable length integers, strings in D (and
> then also BPF) can be stored with their length embedded in the byte
> stream.  This makes it possible to perform operations on strings
> without the need to recalculate the length of the string multiple
> times.
>
> The string constant table stored in DIFOs contains strings that will
> be made available to the BPF program by loading the string constant
> table into a BPF map.  It makes sense to already store strings using
> the variable length prefix when the string constant table is
> constructed, which then also requires code that makes of it to know
> about the variable length integer prefixing each string.

"makes of it" -> "makes use of it"
But what does this last phrase mean?  Is this the same message as that 
of the following paragraph?  Or a different idea?

> This patch introduces a dt_difo_getstr() function that returns a
> regular C char * entity given a DIFO and a string offset into the
> string constant table for that DIFO.  Any code that needs to access
> a string constant from a DIFO is updated to make use of this new
> function.
>
> The string constant table creation code is updated to account for
> the fact that the empty string will now occupy 2 bytes (one for the
> length that is 0, and one for the terminating 0-byte.