[PATCH 3/5] libsframe: rename offset in user-facing sframe_frame_row_entry struct

Indu Bhagat indu.bhagat@oracle.com
Fri Jan 23 10:47:15 GMT 2026


On 1/22/26 2:37 AM, Jens Remus wrote:
> On 1/20/2026 11:28 AM, Indu Bhagat wrote:
>> This patch is the first patch to align libsframe with the terminology
>> change of moving from 'offset' to 'data word'.  With the introduction of
>> flexible FDE type SFRAME_FDE_TYPE_FLEX, the variable-length data
>> following an SFrame FRE header can now represent signed offsets or
>> unsigned control data. Consequently, 'data word' is adopted as the more
>> generic term.
>>
>> This change updates the names used in the user-facing
>> sframe_frame_row_entry structure.  While some API function names remain
>> unchanged to preserve existing contracts, the underlying data buffers
>> and size macros now reflect the data word' terminology.
>>
>> libsframe is a tricky spot for such a terminology change: some of APIs
>> are still used to read (may be followed by endian swap) for dumping
>> SFrame V2 sections in textual format.  Some classic examples are
>> sframe_decode_fre, and flip_fre (both are static functions).  But moving
>> forward, using the term 'data word' for such APIs and their internal too
>> may be better.  Subsequent commits will achieve just that.
>>
>> include/
>> 	* sframe-api.h (MAX_NUM_DATAWORDS): Rename from
>> 	MAX_NUM_STACK_OFFSETS.
>> 	(MAX_DATAWORD_BYTES): Rename from MAX_OFFSET_BYTES.
>> 	(struct sframe_frame_row_entry): Rename fre_offsets to
>> 	fre_datawords.
>> libsframe/
>> 	* sframe.c (sframe_fre_sanity_check_p): Use MAX_NUM_DATAWORDS.
>> 	(sframe_get_fre_offset): Update internal pointers to use
>> 	'offsets' and access fre_datawords.
>> 	(sframe_get_fre_udata): Rename local variables to
>> 	dataword_cnt/dataword_size and update to use
>> 	SFRAME_FRE_DATAWORD_* constants.
>> 	(sframe_decode_fre): Use fre_datawords and MAX_DATAWORD_BYTES.
>> 	(sframe_encoder_add_fre): Use fre_datawords.
>> 	(sframe_encoder_write_fre): Use fre_datawords.
>> ---
>>   include/sframe-api.h |  8 +++----
>>   libsframe/sframe.c   | 50 ++++++++++++++++++++++----------------------
>>   2 files changed, 29 insertions(+), 29 deletions(-)
>>
>> diff --git a/include/sframe-api.h b/include/sframe-api.h
>> index 455738b5ef1..bd297a65395 100644
>> --- a/include/sframe-api.h
>> +++ b/include/sframe-api.h
>> @@ -31,10 +31,10 @@ extern "C"
>>   typedef struct sframe_decoder_ctx sframe_decoder_ctx;
>>   typedef struct sframe_encoder_ctx sframe_encoder_ctx;
>>   
>> -#define MAX_NUM_STACK_OFFSETS	6
>> +#define MAX_NUM_DATAWORDS	6
>>   
>> -#define MAX_OFFSET_BYTES  \
>> -  ((SFRAME_FRE_OFFSET_4B * 2 * MAX_NUM_STACK_OFFSETS))
>> +#define MAX_DATAWORD_BYTES  \
>> +  ((SFRAME_FRE_DATAWORD_4B * 2 * MAX_NUM_DATAWORDS))
>>   
>>   /* User interfacing SFrame Row Entry.
>>      An abstraction provided by libsframe so the consumer is decoupled from
>> @@ -47,7 +47,7 @@ typedef struct sframe_encoder_ctx sframe_encoder_ctx;
>>   typedef struct sframe_frame_row_entry
>>   {
>>     uint32_t fre_start_addr;
>> -  unsigned char fre_offsets[MAX_OFFSET_BYTES];
>> +  unsigned char fre_datawords[MAX_DATAWORD_BYTES];
>>     unsigned char fre_info;
>>   } sframe_frame_row_entry;
>>   
>> diff --git a/libsframe/sframe.c b/libsframe/sframe.c
>> index 52788d8ac15..38baa2277c1 100644
>> --- a/libsframe/sframe.c
>> +++ b/libsframe/sframe.c
>> @@ -465,7 +465,7 @@ sframe_fre_sanity_check_p (const sframe_frame_row_entry *frep)
>>       return false;
>>   
>>     offset_cnt = sframe_fre_get_offset_count (fre_info);
>> -  if (offset_cnt > MAX_NUM_STACK_OFFSETS)
>> +  if (offset_cnt > MAX_NUM_DATAWORDS)
>>       return false;
>>   
>>     return true;
>> @@ -1028,54 +1028,54 @@ sframe_get_fre_offset (const sframe_frame_row_entry *fre, int idx, int *errp)
>>   
>>     if (offset_size == SFRAME_FRE_OFFSET_1B)
>>       {
>> -      int8_t *sp = (int8_t *)fre->fre_offsets;
>> -      return sp[idx];
>> +      int8_t *offsets = (int8_t *)fre->fre_datawords;
>> +      return offsets[idx];
>>       }
>>     else if (offset_size == SFRAME_FRE_OFFSET_2B)
>>       {
>> -      int16_t *sp = (int16_t *)fre->fre_offsets;
>> -      return sp[idx];
>> +      int16_t *offsets = (int16_t *)fre->fre_datawords;
>> +      return offsets[idx];
>>       }
>>     else
>>       {
>> -      int32_t *ip = (int32_t *)fre->fre_offsets;
>> -      return ip[idx];
>> +      int32_t *offsets = (int32_t *)fre->fre_datawords;
>> +      return offsets[idx];
>>       }
>>   }
>>   
>> -/* Get IDX'th offset as unsigned data from FRE.  Set errp as applicable.  */
>> +/* Get IDX'th data word as unsigned data from FRE.  Set errp as applicable.  */
>>   
>>   uint32_t
>>   sframe_get_fre_udata (const sframe_frame_row_entry *fre, int idx, int *errp)
>>   {
>> -  uint8_t offset_cnt, offset_size;
>> +  uint8_t dataword_cnt, dataword_size;
>>   
>>     if (fre == NULL || !sframe_fre_sanity_check_p (fre))
>>       return sframe_set_errno (errp, SFRAME_ERR_FRE_INVAL);
>>   
>> -  offset_cnt = sframe_fre_get_offset_count (fre->fre_info);
>> -  offset_size = sframe_fre_get_offset_size (fre->fre_info);
>> +  dataword_cnt = sframe_fre_get_offset_count (fre->fre_info);
>> +  dataword_size = sframe_fre_get_offset_size (fre->fre_info);
>>   
>> -  if (offset_cnt < idx + 1)
>> +  if (dataword_cnt < idx + 1)
>>       return sframe_set_errno (errp, SFRAME_ERR_FREOFFSET_NOPRESENT);
>>   
>>     if (errp)
>>       *errp = 0; /* Offset Valid.  */
>>   
>> -  if (offset_size == SFRAME_FRE_OFFSET_1B)
>> +  if (dataword_size == SFRAME_FRE_DATAWORD_1B)
>>       {
>> -      uint8_t *offsets = (uint8_t *)fre->fre_offsets;
>> -      return offsets[idx];
>> +      uint8_t *dwords = (uint8_t *)fre->fre_datawords;
>> +      return dwords[idx];
> 
> Nit: Now that datword(s) is used everywhere let's better use that here as well:
> 
>        uint8_t *datawords = (uint8_t *)fre->fre_datawords;
>        return datawords[idx];
> 

Done.

>>       }
>> -  else if (offset_size == SFRAME_FRE_OFFSET_2B)
>> +  else if (dataword_size == SFRAME_FRE_DATAWORD_2B)
>>       {
>> -      uint16_t *offsets = (uint16_t *)fre->fre_offsets;
>> -      return offsets[idx];
>> +      uint16_t *dwords = (uint16_t *)fre->fre_datawords;
>> +      return dwords[idx];
> 
> Likewise.
> 

Done.

>>       }
>>     else
>>       {
>> -      uint32_t *offsets = (uint32_t *)fre->fre_offsets;
>> -      return offsets[idx];
>> +      uint32_t *dwords = (uint32_t *)fre->fre_datawords;
>> +      return dwords[idx];
> 
> Likewise.
> 

Done.

>>       }
>>   }
>>   
>> @@ -1373,13 +1373,13 @@ sframe_decode_fre (const char *fre_buf, sframe_frame_row_entry *fre,
>>     /* Sanity check as the API works closely with the binary format.  */
>>     sframe_assert (sizeof (fre->fre_info) == sizeof (uint8_t));
>>   
>> -  /* Cleanup the space for fre_offsets first, then copy over the valid
>> +  /* Cleanup the space for fre_datawords first, then copy over the valid
>>        bytes.  */
>> -  memset (fre->fre_offsets, 0, MAX_OFFSET_BYTES);
>> +  memset (fre->fre_datawords, 0, MAX_DATAWORD_BYTES);
>>     /* Get offsets size.  */
>>     stack_offsets_sz = sframe_fre_offset_bytes_size (fre->fre_info);
>>     stack_offsets = fre_buf + addr_size + sizeof (fre->fre_info);
>> -  memcpy (fre->fre_offsets, stack_offsets, stack_offsets_sz);
>> +  memcpy (fre->fre_datawords, stack_offsets, stack_offsets_sz);
>>   
>>     /* The FRE has been decoded.  Use it to perform one last sanity check.  */
>>     fre_size = sframe_fre_entry_size (fre, fre_type);
>> @@ -2158,7 +2158,7 @@ sframe_encoder_add_fre (sframe_encoder_ctx *ectx,
>>   
>>     /* frep has already been sanity check'd.  Get offsets size.  */
>>     offsets_sz = sframe_fre_offset_bytes_size (frep->fre_info);
>> -  memcpy (&ectx_frep->fre_offsets, &frep->fre_offsets, offsets_sz);
>> +  memcpy (&ectx_frep->fre_datawords, &frep->fre_datawords, offsets_sz);
>>   
>>     esz = sframe_fre_entry_size (frep, fre_type);
>>     fre_tbl->count++;
>> @@ -2522,7 +2522,7 @@ sframe_encoder_write_fre (char *contents, sframe_frame_row_entry *frep,
>>     memcpy (contents, &frep->fre_info, sizeof (frep->fre_info));
>>     contents += sizeof (frep->fre_info);
>>   
>> -  memcpy (contents, frep->fre_offsets, fre_stack_offsets_sz);
>> +  memcpy (contents, frep->fre_datawords, fre_stack_offsets_sz);
>>     contents+= fre_stack_offsets_sz;
>>   
>>     fre_sz = sframe_fre_entry_size (frep, fre_type);
> 
> Regards,
> Jens



More information about the Binutils mailing list