Re: [PAHOLE Patch v2 1/2] dwarf_loader: Extract die__add_btf_type_tag() helper [NFC]

On 28/05/2026 23:36, Vineet Gupta wrote:
> NFC change preparing for DW_TAG_GNU_annotation support.
> Extract the btf_type_tag annotation creation logic from into a helper
> die__add_btf_type_tag().
> 
> Signed-off-by: Vineet Gupta <[email protected]>
> ---
> Changes since v1 [1]
>  - NFC reinstate some original comments
> 
> [1] https://lore.kernel.org/bpf/[email protected]/
> ---
>  dwarf_loader.c | 58 +++++++++++++++++++++++++++++---------------------
>  1 file changed, 34 insertions(+), 24 deletions(-)
> 
> diff --git a/dwarf_loader.c b/dwarf_loader.c
> index 16fb7becffee..8fc9a6794f2f 100644
> --- a/dwarf_loader.c
> +++ b/dwarf_loader.c
> @@ -1600,14 +1600,45 @@ static struct btf_type_tag_type *die__create_new_btf_type_tag_type(Dwarf_Die *di
>  	return tag;
>  }
>  

looks good Vineet! One small suggestion:

> +static int die__add_btf_type_tag(struct btf_type_tag_ptr_type **tagp,
> +				 Dwarf_Die *die, Dwarf_Die *adie,
> +				 struct cu *cu, struct conf_load *conf)

maybe just change the function to return the struct btf_type_tag_ptr_type *, 
since consumers just check for non-zero rather than doing anything with the
error code?


> +{
> +	struct btf_type_tag_type *annot;
> +	uint32_t id;
> +
> +	if (*tagp == NULL) {
> +		/* Create a btf_type_tag_ptr type. */
> +		*tagp = die__create_new_btf_type_tag_ptr_type(die, cu);
> +		if (!*tagp)
> +			return -ENOMEM;
> +	}
> +
> +	/* Create a btf_type_tag type for this annotation. */
> +	annot = die__create_new_btf_type_tag_type(adie, cu, conf);
> +	if (annot == NULL)
> +		return -ENOMEM;
> +
> +	if (cu__table_add_tag(cu, &annot->tag, &id) < 0)
> +		return -ENOMEM;
> +
> +	struct dwarf_tag *dtag = tag__dwarf(&annot->tag);
> +	dtag->small_id = id;
> +	cu__hash(cu, &annot->tag);
> +
> +	/* For a list of DW_TAG_LLVM_annotation like tag1 -> tag2 -> tag3,
> +	 * the tag->tags contains tag3 -> tag2 -> tag1.
> +	 */
> +	list_add(&annot->node, &(*tagp)->tags);
> +	return 0;
> +}
> +
>  static struct tag *die__create_new_pointer_tag(Dwarf_Die *die, struct cu *cu,
>  					       struct conf_load *conf)
>  {
>  	struct btf_type_tag_ptr_type *tag = NULL;
> -	struct btf_type_tag_type *annot;
>  	Dwarf_Die *cdie, child;
>  	const char *name;
> -	uint32_t id;
>  
>  	/* If no child tags or skipping btf_type_tag encoding, just create a new tag
>  	 * and return
> @@ -1627,29 +1658,8 @@ static struct tag *die__create_new_pointer_tag(Dwarf_Die *die, struct cu *cu,
>  		if (strcmp(name, "btf_type_tag") != 0)
>  			continue;
>  
> -		if (tag == NULL) {
> -			/* Create a btf_type_tag_ptr type. */
> -			tag = die__create_new_btf_type_tag_ptr_type(die, cu);
> -			if (!tag)
> -				return NULL;
> -		}
> -
> -		/* Create a btf_type_tag type for this annotation. */
> -		annot = die__create_new_btf_type_tag_type(cdie, cu, conf);
> -		if (annot == NULL)
> +		if (die__add_btf_type_tag(&tag, die, cdie, cu, conf))
>  			return NULL;
> -
> -		if (cu__table_add_tag(cu, &annot->tag, &id) < 0)
> -			return NULL;
> -
> -		struct dwarf_tag *dtag = tag__dwarf(&annot->tag);
> -		dtag->small_id = id;
> -		cu__hash(cu, &annot->tag);
> -
> -		/* For a list of DW_TAG_LLVM_annotation like tag1 -> tag2 -> tag3,
> -		 * the tag->tags contains tag3 -> tag2 -> tag1.
> -		 */
> -		list_add(&annot->node, &tag->tags);
>  	} while (dwarf_siblingof(cdie, cdie) == 0);
>  
>  	return tag ? &tag->tag : tag__new(die, cu);