On 05.06.2023 12:36, Roger Pau Monne wrote:
> @@ -51,7 +53,7 @@ void libxl_cpuid_dispose(libxl_cpuid_policy_list
> *p_cpuid_list)
> * Used for the static structure describing all features.
> */
> struct cpuid_flags {
> - char* name;
> + const char* name;
Nit: Would you mind also moving * to its designated position at this
occasion?
> @@ -81,6 +83,14 @@ static libxl_cpuid_policy_list
> cpuid_find_match(libxl_cpuid_policy_list *list,
> return *list + i;
> }
>
> +static int search_feature(const void *a, const void *b)
> +{
> + const char *key = a;
> + const char *feat = ((struct { const char *n; } *)b)->n;
Urgh - what a cast. There's no connection at all between this and
struct feature_name in libxl_cpuid_parse_config(). I think you want
to move that type declaration out of the function, to also use it
here. But if not, comments on both sides are going to be necessary
imo.
> + return strncmp(key, feat, strlen(key)) ?: strlen(key) - strlen(feat);
Why not simply
return strcmp(key, feat);
? Both names are nul-terminated, first and foremost because
otherwise using strlen() wouldn't be appropriate either.
> @@ -342,6 +214,28 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list
> *cpuid, const char* str)
> if(!strncmp(str, flag->name, sep - str) && flag->name[sep - str] ==
> 0)
> break;
> }
> + if (flag->name == NULL) {
> + const struct feature_name *feat;
> + /* Provide a NUL terminated feature name to the search helper. */
> + char *name = strndup(str, sep - str);
> +
> + if (name == NULL)
> + return 4;
> +
> + feat = bsearch(name, features, ARRAY_SIZE(features),
> sizeof(features[0]),
> + search_feature);
> + free(name);
> +
> + if (feat != NULL) {
> + f.name = feat->name;
> + f.leaf = feature_to_cpuid[feat->bit / 32].leaf;
> + f.subleaf = feature_to_cpuid[feat->bit / 32].subleaf;
> + f.reg = feature_to_cpuid[feat->bit / 32].reg;
> + f.bit = feat->bit % 32;
> + f.length = 1,
> + flag = &f;
Nit: Too deep indentation throughout here.
Jan
