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

Tue Nov 4 14:32:50 GMT 2025

Hello Indu,

would it make sense to further align the wording of the function
comments as follows, while you are touching them?

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.

>  /* 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
-- 
Jens Remus
Linux on Z Development (D3303)
+49-7031-16-1128 Office
jremus@de.ibm.com

IBM

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/