[PATCH,RESEND 4/4] libsframe: rename encoder to ectx for readability

[Indu Bhagat]

On 11/4/25 6:32 AM, Jens Remus wrote:
> Hello Indu,
> 
> would it make sense to further align the wording of the function
> comments as follows, while you are touching them?
> 

Thanks for reviewing.

I have incorporated your suggestions in V2.

> On 11/3/2025 9:02 PM, Indu Bhagat wrote:
>> Addressing a (old) review comment suggesting this housekeeping item. Use
>> consistent naming style in libsframe.  sframe_decoder_ctx objects are
>> named 'dctx', so use 'ectx' for sframe_encoder_ctx objects.
>>
>> Make necessary changes in both the declaration and definition.
>> ---
>>   include/sframe-api.h |  41 +++++-----
>>   libsframe/sframe.c   | 181 +++++++++++++++++++++----------------------
>>   2 files changed, 111 insertions(+), 111 deletions(-)
>>
>> diff --git a/include/sframe-api.h b/include/sframe-api.h
>> index 375e1decf5e..ae16b0fe8bd 100644
>> --- a/include/sframe-api.h
>> +++ b/include/sframe-api.h
>> @@ -103,10 +103,10 @@ sframe_calc_fre_type (size_t func_size);
>>   
>>   /* The SFrame Decoder.  */
>>   
>> -/* Decode the specified SFrame buffer CF_BUF of size CF_SIZE and return the
>> +/* Decode the specified SFrame buffer SF_BUF of size SF_SIZE and return the
>>      new SFrame decoder context.  Sets ERRP for the caller if any error.  */
>>   extern sframe_decoder_ctx *
>> -sframe_decode (const char *cf_buf, size_t cf_size, int *errp);
>> +sframe_decode (const char *sf_buf, size_t sf_size, int *errp);
> 
> Nit:  This change is not reflected in the commit message.
> 

Sure, I pushed this out as a separate obvious commit.

>>   /* Free the decoder context.  */
>>   extern void
>> @@ -235,62 +235,63 @@ sframe_encode (uint8_t ver, uint8_t flags, uint8_t abi_arch,
>>   
>>   /* Free the encoder context.  */
> 
> Add "ECTXP"?
> 
> /* Free the encoder context ECTXP.  */
> 
>>   extern void
>> -sframe_encoder_free (sframe_encoder_ctx **encoder);
>> +sframe_encoder_free (sframe_encoder_ctx **ectxp);
>>   
>> -/* Get the size of the SFrame header from the encoder ctx ENCODER.  */
>> +/* Get the size of the SFrame header from the encoder ctx ECTX.  */
> 
> Nit: "ctx ECTX" feels like a bit to much abbreviated.
> 
> /* Get the size of the SFrame header from the encoder contect ECTX.  */
> 
>>   extern unsigned int
>> -sframe_encoder_get_hdr_size (sframe_encoder_ctx *encoder);
>> +sframe_encoder_get_hdr_size (sframe_encoder_ctx *ectx);
>>   
>> -/* Get the abi/arch info from the SFrame encoder context CTX.  */
>> +/* Get the abi/arch info from the SFrame encoder context ECTX.  */
> 
> While you reword all of the following comments, would it make sense
> to either always have "SFrame" in "SFrame encoder/decoder context
> ECTX/DCTX" or leave it out or move it as suggested below?
> 
> Move "SFrame"?
> 
> /* Get the SFrame abi/arch info from the encoder context ECTX.  */
> 
>>   extern uint8_t
>> -sframe_encoder_get_abi_arch (sframe_encoder_ctx *encoder);
>> +sframe_encoder_get_abi_arch (sframe_encoder_ctx *ectx);
>>   
>> -/* Get the format version from the SFrame encoder context ENCODER.  */
>> +/* Get the format version from the SFrame encoder context ECTX.  */
> 
> Move "SFrame"?
> 
> /* Get the SFrame format version from the encoder context ECTX.  */
> 
>>   extern uint8_t
>> -sframe_encoder_get_version (sframe_encoder_ctx *encoder);
>> +sframe_encoder_get_version (sframe_encoder_ctx *ectx);
>>   
>> -/* Get the section flags from the SFrame encoder context ENCODER.  */
>> +/* Get the section flags from the SFrame encoder context ECTX.  */
> 
> Move "SFrame"?
> 
> /* Get the SFrame section flags from the encoder context ECTX.  */
> 
>>   extern uint8_t
>> -sframe_encoder_get_flags (sframe_encoder_ctx *encoder);
>> +sframe_encoder_get_flags (sframe_encoder_ctx *ectx);
>>   
>>   /* Get the offset of the sfde_func_start_address field (from the start of the
>>      on-disk layout of the SFrame section) of the FDE at FUNC_IDX in the encoder
>> -   context ENCODER.
>> +   context ECTX.
>>   
>>      If FUNC_IDX is more than the number of SFrame FDEs in the section, sets
>>      error code in ERRP, but returns the (hypothetical) offset.  This is useful
>>      for the linker when arranging input FDEs into the output section to be
>>      emitted.  */
>>   uint32_t
>> -sframe_encoder_get_offsetof_fde_start_addr (sframe_encoder_ctx *encoder,
>> +sframe_encoder_get_offsetof_fde_start_addr (sframe_encoder_ctx *ectx,
>>   					    uint32_t func_idx, int *errp);
>>   
>>   /* Return the number of function descriptor entries in the SFrame encoder
>> -   ENCODER.  */
>> +   ECTX.  */
> 
> Move "SFrame"?
> 
> /* Return the number of SFrame function descriptor entries in the encoder
>     ECTX.  */
> 
>>   extern uint32_t
>> -sframe_encoder_get_num_fidx (sframe_encoder_ctx *encoder);
>> +sframe_encoder_get_num_fidx (sframe_encoder_ctx *ectx);
>>   
>>   /* Add an FRE to function at FUNC_IDX'th function descriptor index entry in
>>      the encoder context.  */
> 
> Add "ECTX"?
> 
> /* Add an FRE to function at FUNC_IDX'th function descriptor index entry in
>     the encoder context ECTX.  */
> 
>>   extern int
>> -sframe_encoder_add_fre (sframe_encoder_ctx *encoder,
>> +sframe_encoder_add_fre (sframe_encoder_ctx *ectx,
>>   			unsigned int func_idx,
>>   			sframe_frame_row_entry *frep);
>>   
>>   /* Add a new function descriptor entry with START_ADDR, FUNC_SIZE, FUNC_INFO
>>      and REP_BLOCK_SIZE to the encoder.  */
> 
> Add "context ECTX"?
> 
> /* Add a new function descriptor entry with START_ADDR, FUNC_SIZE, FUNC_INFO
>     and REP_BLOCK_SIZE to the encoder context ECTX.  */
> 
>>   extern int
>> -sframe_encoder_add_funcdesc_v2 (sframe_encoder_ctx *encoder,
>> +sframe_encoder_add_funcdesc_v2 (sframe_encoder_ctx *ectx,
>>   				int32_t start_addr,
>>   				uint32_t func_size,
>>   				unsigned char func_info,
>>   				uint8_t rep_block_size,
>>   				uint32_t num_fres);
>>   
>> -/* Serialize the contents of the encoder and return the buffer.  ENCODED_SIZE
>> -   is updated to the size of the buffer.  Sets ERRP if failure.  */
>> +/* Serialize the contents of the encoder object ECTX and return the buffer.
> 
> s/object/context/ ?
> 
> /* Serialize the contents of the encoder context ECTX and return the buffer.
> 
>> +   ENCODED_SIZE is updated to the size of the buffer.
>> +   Sets ERRP if failure.  */
>>   extern char  *
>> -sframe_encoder_write (sframe_encoder_ctx *encoder,
>> +sframe_encoder_write (sframe_encoder_ctx *ectx,
>>   		      size_t *encoded_size, int *errp);
>>   
>>   #ifdef	__cplusplus
>> diff --git a/libsframe/sframe.c b/libsframe/sframe.c
> 
> Likewise.
> 
> Regards,
> Jens