[DTrace-devel] [PATCH 10/38] Fix comments in dt_cg.c

[Eugene Loh]

On 7/18/24 15:28, Kris Van Hees wrote:
> On Thu, Jun 27, 2024 at 01:34:27AM -0400, eugene.loh at oracle.com wrote:
>> From: Eugene Loh <eugene.loh at oracle.com>
>>
>> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
>> ---
>>   libdtrace/dt_cg.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
>> index 0977406a..4fd2d359 100644
>> --- a/libdtrace/dt_cg.c
>> +++ b/libdtrace/dt_cg.c
>> @@ -1150,9 +1150,9 @@ dt_cg_prologue(dt_pcb_t *pcb, dt_node_t *pred)
>>   
>>   /*
>>    * Generate the function epilogue:
>> - *	4. Submit the buffer to the perf event output buffer for the current
>> + *	1. Submit the buffer to the perf event output buffer for the current
>>    *	   cpu, if this is a data recording action..
>> - *	5. Return 0
>> + *	2. Return 0
> This numbering was meant to continue the numbering that is in the comment block
> for dt_cg_prologue(), which now would make this be 6 and 7.  The thought behind
> that numbering (which was correctly sequential until we added speculation stuff
> to the prologue) is that 1 through 7 cover all the steps of code that is
> generated for a function.
>
> But if you think that is too comfusing, I am ok with going with 1 and 2 here.
> It just changes what this was meant to convey.

Yeah.  I get the intention, but it's easy to imagine someone tweaking 
numbers in the comment for prologue() not realizing that they also have 
to update comments 100 lines away for a different function.

I'm actually a fan of getting rid of the numbers altogether.  All they 
mean is "in this order," but the order is already clear.  It's not like 
we refer again to these numbers.  So, they're just extra stuff to 
maintain, with no useful value.  (Also, why continue any numbering when 
we don't number the stuff between the prologue and epilogue?  Kind of a 
rhetorical question since that just heads off into some fruitless 
discussion.)

So:

*)  R-b for the patch as is?

*)  R-b for a version of the patch that changes from numbered to 
unnumbered bullets for dt_cg_prologue() and dt_cg_epilogue() comments?

>
>>    * }
>>    */
>>   static void
>> -- 
>> 2.18.4
>>