Re: [RFC dwarves 3/4] pahole: add --btf_features=feature1[,feature2...] support

[Eduard Zingerman <eddyz87@gmail.com>]

On Wed, 2023-10-11 at 17:41 +0100, Alan Maguire wrote:
[...]
> > > +		}
> > > +	}
> > > +
> > > +	for (i = 0; i < ARRAY_SIZE(btf_features); i++) {
> > > +		bool *bval = (bool *)(((void *)conf_load) + btf_features[i].conf_load_offset);
> > > +		bool match = encode_all;
> > > +
> > > +		if (!match) {
> > > +			for (j = 0; j < n; j++) {
> > > +				if (strcmp(feature_list[j], btf_features[i].name) == 0) {
> > > +					match = true;
> > > +					break;
> > > +				}
> > > +			}
> > > +		}
> > > +		if (match)
> > > +			*bval = btf_features[i].skip ? false : true;
> > 
> > I'm not sure I understand the logic behind "skip" features.
> > Take `decl_tag` for example:
> > - by default conf_load->skip_encoding_btf_decl_tag is 0;
> > - if `--btf_features=decl_tag` is passed it is still 0 because of the
> >   `skip ? false : true` logic.
> > 
> > If there is no way to change "skip" features why listing these at all?
> > 
> You're right; in the case of a skip feature, I think we need the
> following behaviour
> 
> 1. we skip the encoding by default (so the equivalent of
> --skip_encoding_btf_decl_tag, setting skip_encoding_btf_decl_tag
> to true
>
> 2. if the user however specifies the logical inversion of the skip
> feature in --btf_features (in this case "decl_tag" - or "all")
> skip_encoding_btf_decl_tag is set to false.
> 
> So in my code we had 2 above but not 1. If both were in place I think
> we'd have the right set of behaviours. Does that sound right?

You mean when --features=? is specified we default to
conf_load->skip_encoding_btf_decl_tag = true, and set it to false only
if "all" or "decl_tag" is listed in features, right?
 
> Maybe a better way to express all this would be to rename the "skip"
> field in "struct btf_feature" to "default" - so in the case of a "skip"
> feature, the default is true, but for opt-in features, the default is false.

Yes, I agree, "default" is better as "skip" is a bit confusing.

[...]