[DTrace-devel] [PATCH v2 2/4] dt_impl: bump number of TSLOTS to 8

[Alan Maguire]

On 01/07/2025 19:31, Eugene Loh wrote:
> Very simple patch, but one key problem.  The changed line is preceded by
> a huge comment block that goes to excruciating pains to explain why the
> value should be 4.  So, I'm fine with the change itself and I think the
> "Because of the complexity..." sentence in the commit message can be
> dropped, but the comment block in the file needs to be updated to
> explain (with corresponding pains?) why we need 8 (not 9, not 7).
>

Ah good catch, forgot to do this. I actually think 6 is enough.
The additional seems to be a result of the various complex inet_ntoa*()
calls in the tcp.d translators; some have ternary operators with
multiple inet_ntoa*()s. The origin appears to be dt_cg_ternary_op() and
that has the comment:

        /*
         * Strings complicate things a bit because dn_left and dn_right
might
         * actually be temporary strings (tstring) *and* in different slots.
         * We need to allocate a new tstring to hold the result, and
copy the
         * value into the new tstring (and free any tstrings in dn_left and
         * dn_right).
         */

So if we have a left and right ternary and both allocate 3 tstring slots
for inet_ntoa*()s that means 3*2 = 6 tstring slots should actually be
enough. Additional testing suggests that is so; does the above make
sense or are there additional things to consider here? I'll update the
next version with a comment describing the above anyhow. Thanks for the
review!

Alan


> On 6/10/25 09:58, Alan Maguire wrote:
>> Because of the complexity of the TCP translators, more tslots are
>> needed.
>>
>> Signed-off-by: Alan Maguire <alan.maguire at oracle.com>
>> ---
>>   libdtrace/dt_impl.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
>> index 68fb8ec5..10424f9c 100644
>> --- a/libdtrace/dt_impl.h
>> +++ b/libdtrace/dt_impl.h
>> @@ -218,7 +218,7 @@ typedef struct dt_kern_path {
>>    * - cleanpath() holds a prepended '/' char, a string, an appended
>> '/' char,
>>    *   and a terminating NUL char, or STRSZ + 3 chars altogether
>>    */
>> -#define DT_TSTRING_SLOTS    4
>> +#define DT_TSTRING_SLOTS    8
>>   #define DT_TSTRING_SIZE(dtp)    \
>>           MAX(P2ROUNDUP((dtp)->dt_options[DTRACEOPT_STRSIZE] + 3, 8), \
>>               72)