Message ID | 1252621257-26364-10-git-send-email-andre.przywara@amd.com |
---|---|
State | Superseded |
Headers | show |
On (Fri) Sep 11 2009 [00:20:54], Andre Przywara wrote: > avoid code duplication and handle the CPUID flag name search in a > loop. > > Signed-off-by: Andre Przywara <andre.przywara@amd.com> > --- > target-i386/cpuid.c | 33 +++++++++++++-------------------- > 1 files changed, 13 insertions(+), 20 deletions(-) > > diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c > index e62dc04..4be1449 100644 > --- a/target-i386/cpuid.c > +++ b/target-i386/cpuid.c > @@ -71,29 +71,22 @@ static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features, > uint32_t *ext2_features, > uint32_t *ext3_features) > { > - int i; > + int i, j; > int found = 0; > + const char ** feature_names[4] = {feature_name, ext_feature_name, > + ext2_feature_name, ext3_feature_name}; > + uint32_t* feature_flags[4] = {features, ext_features, > + ext2_features, ext3_features}; > > - for ( i = 0 ; i < 32 ; i++ ) > - if (feature_name[i] && !strcmp (flagname, feature_name[i])) { > - *features |= 1 << i; > - found = 1; > - } > - for ( i = 0 ; i < 32 ; i++ ) > - if (ext_feature_name[i] && !strcmp (flagname, ext_feature_name[i])) { > - *ext_features |= 1 << i; > - found = 1; > - } > - for ( i = 0 ; i < 32 ; i++ ) > - if (ext2_feature_name[i] && !strcmp (flagname, ext2_feature_name[i])) { > - *ext2_features |= 1 << i; > - found = 1; > - } > - for ( i = 0 ; i < 32 ; i++ ) > - if (ext3_feature_name[i] && !strcmp (flagname, ext3_feature_name[i])) { > - *ext3_features |= 1 << i; > - found = 1; > + for (j = 0; j < 4; j++) { > + for (i = 0; i < 32; i++) { > + if (feature_names[j][i] && > + !strcmp(flagname, feature_names[j][i])) { > + *feature_flags[j] |= 1 << i; > + found = 1; > + } > } > + } > if (!found) { > fprintf(stderr, "CPU feature %s not found\n", flagname); > } This just reports the entire string, right? Not just the feature as the printf suggests. Amit
Amit Shah wrote: > On (Fri) Sep 11 2009 [00:20:54], Andre Przywara wrote: >> avoid code duplication and handle the CPUID flag name search in a >> loop. >> --- a/target-i386/cpuid.c >> +++ b/target-i386/cpuid.c >> @@ -71,29 +71,22 @@ static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features, >> uint32_t *ext2_features, >> uint32_t *ext3_features) >> { >>... >> + for (j = 0; j < 4; j++) { >> + for (i = 0; i < 32; i++) { >> + if (feature_names[j][i] && >> + !strcmp(flagname, feature_names[j][i])) { >> + *feature_flags[j] |= 1 << i; >> + found = 1; >> + } >> + } >> if (!found) { >> fprintf(stderr, "CPU feature %s not found\n", flagname); >> } > > This just reports the entire string, right? Not just the feature as the > printf suggests. What makes you think so? flagname is just the single flag (being returned by strtok in the caller, wich null-terminates its results). Otherwise the strcmp() above would not work either... Unknown flags will be reported, but do not abort QEMU (I checked this myself by accident ;-) Regards, Andre.
On (Fri) Sep 11 2009 [21:30:52], Andre Przywara wrote: > Amit Shah wrote: >> On (Fri) Sep 11 2009 [00:20:54], Andre Przywara wrote: >>> avoid code duplication and handle the CPUID flag name search in a >>> loop. >>> --- a/target-i386/cpuid.c >>> +++ b/target-i386/cpuid.c >>> @@ -71,29 +71,22 @@ static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features, >>> uint32_t *ext2_features, >>> uint32_t *ext3_features) >>> { > >>... >>> + for (j = 0; j < 4; j++) { >>> + for (i = 0; i < 32; i++) { >>> + if (feature_names[j][i] && >>> + !strcmp(flagname, feature_names[j][i])) { >>> + *feature_flags[j] |= 1 << i; >>> + found = 1; >>> + } >>> + } >>> if (!found) { >>> fprintf(stderr, "CPU feature %s not found\n", flagname); >>> } >> >> This just reports the entire string, right? Not just the feature as the >> printf suggests. > What makes you think so? flagname is just the single flag (being > returned by strtok in the caller, wich null-terminates its results). > Otherwise the strcmp() above would not work either... > Unknown flags will be reported, but do not abort QEMU (I checked this > myself by accident ;-) strtok, right. It'd be good to remove that. Esp with QemuOpts it would be easier Amit
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c index e62dc04..4be1449 100644 --- a/target-i386/cpuid.c +++ b/target-i386/cpuid.c @@ -71,29 +71,22 @@ static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features, uint32_t *ext2_features, uint32_t *ext3_features) { - int i; + int i, j; int found = 0; + const char ** feature_names[4] = {feature_name, ext_feature_name, + ext2_feature_name, ext3_feature_name}; + uint32_t* feature_flags[4] = {features, ext_features, + ext2_features, ext3_features}; - for ( i = 0 ; i < 32 ; i++ ) - if (feature_name[i] && !strcmp (flagname, feature_name[i])) { - *features |= 1 << i; - found = 1; - } - for ( i = 0 ; i < 32 ; i++ ) - if (ext_feature_name[i] && !strcmp (flagname, ext_feature_name[i])) { - *ext_features |= 1 << i; - found = 1; - } - for ( i = 0 ; i < 32 ; i++ ) - if (ext2_feature_name[i] && !strcmp (flagname, ext2_feature_name[i])) { - *ext2_features |= 1 << i; - found = 1; - } - for ( i = 0 ; i < 32 ; i++ ) - if (ext3_feature_name[i] && !strcmp (flagname, ext3_feature_name[i])) { - *ext3_features |= 1 << i; - found = 1; + for (j = 0; j < 4; j++) { + for (i = 0; i < 32; i++) { + if (feature_names[j][i] && + !strcmp(flagname, feature_names[j][i])) { + *feature_flags[j] |= 1 << i; + found = 1; + } } + } if (!found) { fprintf(stderr, "CPU feature %s not found\n", flagname); }
avoid code duplication and handle the CPUID flag name search in a loop. Signed-off-by: Andre Przywara <andre.przywara@amd.com> --- target-i386/cpuid.c | 33 +++++++++++++-------------------- 1 files changed, 13 insertions(+), 20 deletions(-)