Re: [PATCH bpf-next v2] bpf: Better build_id caching in stack_map_get_build_id_offset()

[Ihor Solodrai <ihor.solodrai@linux.dev>]

On 6/12/26 6:45 PM, Alexei Starovoitov wrote:
> On Fri Jun 12, 2026 at 3:06 PM PDT, Ihor Solodrai wrote:
>> +static void stack_map_build_id_cache_set_resolved(struct stack_map_build_id_cache *cache,
>> +						  struct file *file,
>> +						  const unsigned char *build_id,
>> +						  unsigned long vm_start,
>> +						  unsigned long vm_end,
>> +						  unsigned long vm_pgoff)
>> +{
>> +	if (cache->resolved.file)
>> +		fput(cache->resolved.file);
>> +	cache->resolved.file = file;
>> +	cache->resolved.build_id = build_id;
>> +	cache->resolved.vm_start = vm_start;
>> +	cache->resolved.vm_end = vm_end;
>> +	cache->resolved.vm_pgoff = vm_pgoff;
>> +}
> 
> sorry for broken record.
> claude's copy paste doesn't bother you?

tbh, this particular block is fine with me, it's certainly
not as bad as big expressions in the conditions that I rewrote
in the other helper. I guess I'm just used to lengthy qualified
expressions from my java days, so it doesn't bother me much
(maybe it should).

I briefly chatted with Eduard off-list about this, and I agree
with him that what makes this patch awkward is anonymous structs
as cache fields. I'll try something else in v3.

> It could have been:
> 
> struct resolved *res = &cache->resolved;
> if (res->file)
> 	fput(res->file);
> res->file = file;
> res->build_id = build_id;
> res->vm_start = vm_start;
> res->vm_end = vm_end;
> res->vm_pgoff = vm_pgoff;
> 
> or
> 
> cache->resolved = (struct resolved) { file, build_iud, vm_start, emnv_end, vm_pgoff };
> 
> and probably other options to make a code easier for humans to read.
> 
> You have to explictly tell claude to deduplicate otherwise it keeps
> copy pasting left and right.

I am painfully aware of claude's quirks, yes :)