| Message ID | 20240405032507.2637311-9-david.e.box@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Intel On Demand changes | expand |
On Thu, 4 Apr 2024, David E. Box wrote: > Use printf width specifier to set the display length of encoded feature > names. > > Signed-off-by: David E. Box <david.e.box@linux.intel.com> > Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > --- > > V3 - Add FEAT_LEN #def > > V2 - Split of V1 patch 7 > > tools/arch/x86/intel_sdsi/intel_sdsi.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/tools/arch/x86/intel_sdsi/intel_sdsi.c b/tools/arch/x86/intel_sdsi/intel_sdsi.c > index 45bc69e6718e..0c9670ba1f15 100644 > --- a/tools/arch/x86/intel_sdsi/intel_sdsi.c > +++ b/tools/arch/x86/intel_sdsi/intel_sdsi.c > @@ -43,6 +43,7 @@ > #define METER_CERT_MAX_SIZE 4096 > #define STATE_MAX_NUM_LICENSES 16 > #define STATE_MAX_NUM_IN_BUNDLE (uint32_t)8 > +#define FEAT_LEN 4 > > #define __round_mask(x, y) ((__typeof__(x))((y) - 1)) > #define round_up(x, y) ((((x) - 1) | __round_mask(x, y)) + 1) > @@ -409,11 +410,10 @@ static int sdsi_meter_cert_show(struct sdsi_dev *s) > > printf("Number of Feature Counters: %ld\n", BUNDLE_COUNT(mc->bundle_length)); > while (count < BUNDLE_COUNT(mc->bundle_length)) { > - char feature[5]; > + char feature[FEAT_LEN]; > > - feature[4] = '\0'; > get_feature(bec[count].encoding, feature); > - printf(" %s: %d\n", feature, bec[count].counter); > + printf(" %.4s: %d\n", feature, bec[count].counter); > ++count; > } > > @@ -494,7 +494,7 @@ static int sdsi_state_cert_show(struct sdsi_dev *s) > sizeof(*lki) + // size of the license key info > offset; // offset to this blob content > struct bundle_encoding *bundle = (void *)(lbc) + sizeof(*lbc); > - char feature[5]; > + char feature[FEAT_LEN]; > uint32_t i; > > printf(" Blob %d:\n", count - 1); > @@ -507,11 +507,9 @@ static int sdsi_state_cert_show(struct sdsi_dev *s) > printf(" Blob revision ID: %u\n", lbc->rev_id); > printf(" Number of Features: %u\n", lbc->num_bundles); > > - feature[4] = '\0'; > - > for (i = 0; i < min(lbc->num_bundles, STATE_MAX_NUM_IN_BUNDLE); i++) { > get_feature(bundle[i].encoding, feature); > - printf(" Feature %d: %s\n", i, feature); > + printf(" Feature %d: %.4s\n", i, feature); > } > > if (lbc->num_bundles > STATE_MAX_NUM_IN_BUNDLE) > Hi, After staring this for a while, I cannot get rid of the feeling that the removal of NUL termination is a step into wrong direction. But IMO, instead of the caller side, the NUL termination could be added inside get_feature().
On Mon, 2024-04-08 at 20:22 +0300, Ilpo Järvinen wrote: > On Thu, 4 Apr 2024, David E. Box wrote: > > > Use printf width specifier to set the display length of encoded feature > > names. > > > > Signed-off-by: David E. Box <david.e.box@linux.intel.com> > > Reviewed-by: Kuppuswamy Sathyanarayanan > > <sathyanarayanan.kuppuswamy@linux.intel.com> > > --- > > > > V3 - Add FEAT_LEN #def > > > > V2 - Split of V1 patch 7 > > > > tools/arch/x86/intel_sdsi/intel_sdsi.c | 13 ++++++------- > > 1 file changed, 6 insertions(+), 7 deletions(-) > > > > diff --git a/tools/arch/x86/intel_sdsi/intel_sdsi.c > > b/tools/arch/x86/intel_sdsi/intel_sdsi.c > > index 45bc69e6718e..0c9670ba1f15 100644 > > --- a/tools/arch/x86/intel_sdsi/intel_sdsi.c > > +++ b/tools/arch/x86/intel_sdsi/intel_sdsi.c > > @@ -43,6 +43,7 @@ > > #define METER_CERT_MAX_SIZE 4096 > > #define STATE_MAX_NUM_LICENSES 16 > > #define STATE_MAX_NUM_IN_BUNDLE (uint32_t)8 > > +#define FEAT_LEN 4 > > > > #define __round_mask(x, y) ((__typeof__(x))((y) - 1)) > > #define round_up(x, y) ((((x) - 1) | __round_mask(x, y)) + 1) > > @@ -409,11 +410,10 @@ static int sdsi_meter_cert_show(struct sdsi_dev *s) > > > > printf("Number of Feature Counters: %ld\n", BUNDLE_COUNT(mc- > > >bundle_length)); > > while (count < BUNDLE_COUNT(mc->bundle_length)) { > > - char feature[5]; > > + char feature[FEAT_LEN]; > > > > - feature[4] = '\0'; > > get_feature(bec[count].encoding, feature); > > - printf(" %s: %d\n", feature, > > bec[count].counter); > > + printf(" %.4s: %d\n", feature, > > bec[count].counter); > > ++count; > > } > > > > @@ -494,7 +494,7 @@ static int sdsi_state_cert_show(struct sdsi_dev *s) > > sizeof(*lki) + // size of the > > license key info > > offset; // offset > > to this blob content > > struct bundle_encoding *bundle = (void *)(lbc) + > > sizeof(*lbc); > > - char feature[5]; > > + char feature[FEAT_LEN]; > > uint32_t i; > > > > printf(" Blob %d:\n", count - 1); > > @@ -507,11 +507,9 @@ static int sdsi_state_cert_show(struct sdsi_dev *s) > > printf(" Blob revision ID: %u\n", lbc- > > >rev_id); > > printf(" Number of Features: %u\n", lbc- > > >num_bundles); > > > > - feature[4] = '\0'; > > - > > for (i = 0; i < min(lbc->num_bundles, > > STATE_MAX_NUM_IN_BUNDLE); i++) { > > get_feature(bundle[i].encoding, feature); > > - printf(" Feature %d: %s\n", > > i, feature); > > + printf(" Feature %d: > > %.4s\n", i, feature); > > } > > > > if (lbc->num_bundles > STATE_MAX_NUM_IN_BUNDLE) > > > > Hi, > > After staring this for a while, I cannot get rid of the feeling that the > removal of NUL termination is a step into wrong direction. But IMO, > instead of the caller side, the NUL termination could be added inside > get_feature(). Yeah, you're right. I'll make this change. David >
diff --git a/tools/arch/x86/intel_sdsi/intel_sdsi.c b/tools/arch/x86/intel_sdsi/intel_sdsi.c index 45bc69e6718e..0c9670ba1f15 100644 --- a/tools/arch/x86/intel_sdsi/intel_sdsi.c +++ b/tools/arch/x86/intel_sdsi/intel_sdsi.c @@ -43,6 +43,7 @@ #define METER_CERT_MAX_SIZE 4096 #define STATE_MAX_NUM_LICENSES 16 #define STATE_MAX_NUM_IN_BUNDLE (uint32_t)8 +#define FEAT_LEN 4 #define __round_mask(x, y) ((__typeof__(x))((y) - 1)) #define round_up(x, y) ((((x) - 1) | __round_mask(x, y)) + 1) @@ -409,11 +410,10 @@ static int sdsi_meter_cert_show(struct sdsi_dev *s) printf("Number of Feature Counters: %ld\n", BUNDLE_COUNT(mc->bundle_length)); while (count < BUNDLE_COUNT(mc->bundle_length)) { - char feature[5]; + char feature[FEAT_LEN]; - feature[4] = '\0'; get_feature(bec[count].encoding, feature); - printf(" %s: %d\n", feature, bec[count].counter); + printf(" %.4s: %d\n", feature, bec[count].counter); ++count; } @@ -494,7 +494,7 @@ static int sdsi_state_cert_show(struct sdsi_dev *s) sizeof(*lki) + // size of the license key info offset; // offset to this blob content struct bundle_encoding *bundle = (void *)(lbc) + sizeof(*lbc); - char feature[5]; + char feature[FEAT_LEN]; uint32_t i; printf(" Blob %d:\n", count - 1); @@ -507,11 +507,9 @@ static int sdsi_state_cert_show(struct sdsi_dev *s) printf(" Blob revision ID: %u\n", lbc->rev_id); printf(" Number of Features: %u\n", lbc->num_bundles); - feature[4] = '\0'; - for (i = 0; i < min(lbc->num_bundles, STATE_MAX_NUM_IN_BUNDLE); i++) { get_feature(bundle[i].encoding, feature); - printf(" Feature %d: %s\n", i, feature); + printf(" Feature %d: %.4s\n", i, feature); } if (lbc->num_bundles > STATE_MAX_NUM_IN_BUNDLE)