On 12/10/2023 13:53, Jiri Olsa wrote:
> On Wed, Oct 11, 2023 at 10:17:31AM +0100, Alan Maguire wrote:
>
> SNIP
>
>> +#define BTF_FEATURE(name, alias, skip) \
>> + { #name, #alias, offsetof(struct conf_load, alias), skip }
>> +
>> +struct btf_feature {
>> + const char *name;
>> + const char *option_alias;
>> + size_t conf_load_offset;
>> + bool skip;
>> +} btf_features[] = {
>> + BTF_FEATURE(encode_force, btf_encode_force, false),
>> + BTF_FEATURE(var, skip_encoding_btf_vars, true),
>> + BTF_FEATURE(float, btf_gen_floats, false),
>> + BTF_FEATURE(decl_tag, skip_encoding_btf_decl_tag, true),
>> + BTF_FEATURE(type_tag, skip_encoding_btf_type_tag, true),
>> + BTF_FEATURE(enum64, skip_encoding_btf_enum64, true),
>> + BTF_FEATURE(optimized, btf_gen_optimized, false),
>> + /* the "skip" in skip_encoding_btf_inconsistent_proto is misleading
>> + * here; this is a positive feature to ensure consistency of
>> + * representation rather than a negative option which we want
>> + * to invert. So as a result, "skip" is false here.
>> + */
>> + BTF_FEATURE(consistent, skip_encoding_btf_inconsistent_proto, false),
>> +};
>> +
>> +#define BTF_MAX_FEATURES 32
>> +#define BTF_MAX_FEATURE_STR 256
>> +
>> +/* Translate --btf_features=feature1[,feature2] into conf_load values.
>> + * Explicitly ignores unrecognized features to allow future specification
>> + * of new opt-in features.
>> + */
>> +static void parse_btf_features(const char *features, struct conf_load *conf_load)
>> +{
>> + char *feature_list[BTF_MAX_FEATURES] = {};
>> + char f[BTF_MAX_FEATURE_STR];
>> + bool encode_all = false;
>> + int i, j, n = 0;
>> +
>> + strncpy(f, features, sizeof(f));
>> +
>> + if (strcmp(features, "all") == 0) {
>> + encode_all = true;
>> + } else {
>> + char *saveptr = NULL, *s = f, *t;
>> +
>> + while ((t = strtok_r(s, ",", &saveptr)) != NULL) {
>> + s = NULL;
>> + feature_list[n++] = t;
>> + }
>> + }
>> +
>> + for (i = 0; i < ARRAY_SIZE(btf_features); i++) {
>> + bool *bval = (bool *)(((void *)conf_load) + btf_features[i].conf_load_offset);
>
> nit, would it be easier to have btf_features defined inside the function
> and pass specific bool pointers directly to BTF_FEATURE macro?
>
thanks for taking a look! I _think_ I see what you mean; if we had
conf_load we could encode the bool pointer directly using
the BTF_FEATURE() definition, something like
#define BTF_FEATURE(name, alias, default_value) \
{ #name, #alias, &conf_load->alias, default_value }
struct btf_feature {
const char *name;
const char *option_alias;
bool *conf_value;
bool default_value;
} btf_features[] = {
...
This will work I think because conf_load is a global variable,
and I think we need to keep it global since it's also used by
patch 4 to get the list of supported features. Is the above
something like what you had in mind? Thanks!
Alan
> jirka
>
>> + 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;
>> + }
>> +}
>>
>> static const struct argp_option pahole__options[] = {
>> {
>> @@ -1651,6 +1728,12 @@ static const struct argp_option pahole__options[] = {
>> .key = ARGP_skip_encoding_btf_inconsistent_proto,
>> .doc = "Skip functions that have multiple inconsistent function prototypes sharing the same name, or that use unexpected registers for parameter values."
>> },
>> + {
>> + .name = "btf_features",
>> + .key = ARGP_btf_features,
>> + .arg = "FEATURE_LIST",
>> + .doc = "Specify supported BTF features in FEATURE_LIST or 'all' for all supported features. See the pahole manual page for the list of supported features."
>> + },
>> {
>> .name = NULL,
>> }
>> @@ -1796,7 +1879,7 @@ static error_t pahole__options_parser(int key, char *arg,
>> case ARGP_btf_gen_floats:
>> conf_load.btf_gen_floats = true; break;
>> case ARGP_btf_gen_all:
>> - conf_load.btf_gen_floats = true; break;
>> + parse_btf_features("all", &conf_load); break;
>> case ARGP_with_flexible_array:
>> show_with_flexible_array = true; break;
>> case ARGP_prettify_input_filename:
>> @@ -1826,6 +1909,8 @@ static error_t pahole__options_parser(int key, char *arg,
>> conf_load.btf_gen_optimized = true; break;
>> case ARGP_skip_encoding_btf_inconsistent_proto:
>> conf_load.skip_encoding_btf_inconsistent_proto = true; break;
>> + case ARGP_btf_features:
>> + parse_btf_features(arg, &conf_load); break;
>> default:
>> return ARGP_ERR_UNKNOWN;
>> }
>> --
>> 2.31.1
>>
>
