Received-SPF: pass (google.com: domain of linux-kernel+bounces-135968-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <41b41cfbd79d8f9d0a9dfd187710015021fae252.camel@linux.intel.com>
Subject: Re: [PATCH V3 8/9] platform/x86/intel/sdsi: Simplify ascii printing
From: "David E. Box" <david.e.box@linux.intel.com>
Reply-To: david.e.box@linux.intel.com
To: Ilpo =?ISO-8859-1?Q?J=E4rvinen?= <ilpo.jarvinen@linux.intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>,
 platform-driver-x86@vger.kernel.org,  LKML <linux-kernel@vger.kernel.org>,
 sathyanarayanan.kuppuswamy@linux.intel.com
Date: Mon, 08 Apr 2024 15:11:49 -0700
In-Reply-To: <6dd8b2c0-1c89-43f0-0426-df8d483a7b1e@linux.intel.com>
References: <20240405032507.2637311-1-david.e.box@linux.intel.com>
	 <20240405032507.2637311-9-david.e.box@linux.intel.com>
	 <6dd8b2c0-1c89-43f0-0426-df8d483a7b1e@linux.intel.com>
Autocrypt: addr=david.e.box@linux.intel.com; prefer-encrypt=mutual;
 keydata=mQENBF2w2YABCACw5TpqmFTR6SgsrNqZE8ro1q2lUgVZda26qIi8GeHmVBmu572RfPydisEpCK246rYM5YY9XAps810ZxgFlLyBqpE/rxB4Dqvh04QePD6fQNui/QCSpyZ6j9F8zl0zutOjfNTIQBkcar28hazL9I8CGnnMko21QDl4pkrq1dgLSgl2r2N1a6LJ2l8lLnQ1NJgPAev4BWo4WAwH2rZ94aukzAlkFizjZXmB/6em+lhinTR9hUeXpTwcaAvmCHmrUMxeOyhx+csO1uAPUjxL7olj2J83dv297RrpjMkDyuUOv8EJlPjvVogJF1QOd5MlkWdj+6vnVDRfO8zUwm2pqg25DABEBAAG0KkRhdmlkIEUuIEJveCA8ZGF2aWQuZS5ib3hAbGludXguaW50ZWwuY29tPokBTgQTAQgAOBYhBBFoZ8DYRC+DyeuV6X7Mry1gl3p/BQJdsNmAAhsDBQsJCAcCBhUKCQgLAgQWAgMBAh4BAheAAAoJEH7Mry1gl3p/NusIAK9z1xnXphedgZMGNzifGUs2UUw/xNl91Q9qRaYGyNYATI6E7zBYmynsUL/4yNFnXK8P/I7WMffiLoMqmUvNp9pG6oYYj8ouvbCexS21jgw54I3m61M+wTokieRIO/GettVlCGhz7YHlHtGGqhzzWB3CGPSJMwsouDPvyFFE+28p5d2v9l6rXSb7T297Kh50VX9Ele8QEKngrG+Z/u2lr/bHEhvx24vI8ka22cuTaZvThYMwLTSC4kq9L9WgRv31JBSa1pcbcHLOCoUl0RaQwe6J8w9hN2uxCssHrrfhSA4YjxKNIIp3YH4IpvzuDR3AadYz1klFTnEOxIM7fvQ2iGu5AQ0EXbDZgAEIAPGbL3wvbYUDGMoBSN89GtiC6ybWo28JSiYIN5N9LhDTwfWROenkRvmTESaE5fAM24sh8S0h+F+eQ7j/E/RF3pM31gSovTKw0Pxk7GorK
	FSa25CWemxSV97zV8fVegGkgfZkBMLUId+AYCD1d2R+tndtgjrHtVq/AeN0N09xv/d3a+Xzc4ib/SQh9mM50ksqiDY70EDe8hgPddYH80jHJtXFVA7Ar1ew24TIBF2rxYZQJGLe+Mt2zAzxOYeQTCW7WumD/ZoyMm7bg46/2rtricKnpaACM7M0r7g+1gUBowFjF4gFqY0tbLVQEB/H5e9We/C2zLG9r5/Lt22dj7I8A6kAEQEAAYkBNgQYAQgAIBYhBBFoZ8DYRC+DyeuV6X7Mry1gl3p/BQJdsNmAAhsMAAoJEH7Mry1gl3p/Z/AH/Re8YwzY5I9ByPM56B3Vkrh8qihZjsF7/WB14Ygl0HFzKSkSMTJ+fvZv19bk3lPIQi5lUBuU5rNruDNowCsnvXr+sFxFyTbXw0AQXIsnX+EkMg/JO+/V/UszZiqZPkvHsQipCFVLod/3G/yig9RUO7A/1efRi0E1iJAa6qHrPqE/kJANbz/x+9wcx1VfFwraFXbdT/P2JeOcW/USW89wzMRmOo+AiBSnTI4xvb1s/TxSfoLZvtoj2MR+2PW1zBALWYUKHOzhfFKs3cMufwIIoQUPVqGVeH+u6Asun6ZpNRxdDONop+uEXHe6q6LzI/NnczqoZQLhM8d1XqokYax/IZ4=
Organization: David E. Box
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.50.4 (3.50.4-1.fc39) 
Precedence: bulk
MIME-Version: 1.0

On Mon, 2024-04-08 at 20:22 +0300, Ilpo J=C3=A4rvinen wrote:
> On Thu, 4 Apr 2024, David E. Box wrote:
>=20
> > Use printf width specifier to set the display length of encoded feature
> > names.
> >=20
> > Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> > Reviewed-by: Kuppuswamy Sathyanarayanan
> > <sathyanarayanan.kuppuswamy@linux.intel.com>
> > ---
> >=20
> > V3 - Add FEAT_LEN #def
> >=20
> > V2 - Split of V1 patch 7
> >=20
> > =C2=A0tools/arch/x86/intel_sdsi/intel_sdsi.c | 13 ++++++-------
> > =C2=A01 file changed, 6 insertions(+), 7 deletions(-)
> >=20
> > 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 @@
> > =C2=A0#define METER_CERT_MAX_SIZE	4096
> > =C2=A0#define STATE_MAX_NUM_LICENSES	16
> > =C2=A0#define STATE_MAX_NUM_IN_BUNDLE	(uint32_t)8
> > +#define FEAT_LEN		4
> > =C2=A0
> > =C2=A0#define __round_mask(x, y) ((__typeof__(x))((y) - 1))
> > =C2=A0#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)
> > =C2=A0
> > =C2=A0	printf("Number of Feature Counters:=C2=A0=C2=A0 %ld\n", BUNDLE_C=
OUNT(mc-
> > >bundle_length));
> > =C2=A0	while (count < BUNDLE_COUNT(mc->bundle_length)) {
> > -		char feature[5];
> > +		char feature[FEAT_LEN];
> > =C2=A0
> > -		feature[4] =3D '\0';
> > =C2=A0		get_feature(bec[count].encoding, feature);
> > -		printf("=C2=A0=C2=A0=C2=A0 %s:=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0 %d\n", feature,
> > bec[count].counter);
> > +		printf("=C2=A0=C2=A0=C2=A0 %.4s:=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0 %d\n", feature,
> > bec[count].counter);
> > =C2=A0		++count;
> > =C2=A0	}
> > =C2=A0
> > @@ -494,7 +494,7 @@ static int sdsi_state_cert_show(struct sdsi_dev *s)
> > =C2=A0			sizeof(*lki) +			// size of the
> > license key info
> > =C2=A0			offset;				// offset
> > to this blob content
> > =C2=A0		struct bundle_encoding *bundle =3D (void *)(lbc) +
> > sizeof(*lbc);
> > -		char feature[5];
> > +		char feature[FEAT_LEN];
> > =C2=A0		uint32_t i;
> > =C2=A0
> > =C2=A0		printf("=C2=A0=C2=A0=C2=A0=C2=A0 Blob %d:\n", count - 1);
> > @@ -507,11 +507,9 @@ static int sdsi_state_cert_show(struct sdsi_dev *s=
)
> > =C2=A0		printf("=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Blob revisio=
n ID:=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 %u\n", lb=
c-
> > >rev_id);
> > =C2=A0		printf("=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Number of Fe=
atures:=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 %u\n", lbc-
> > >num_bundles);
> > =C2=A0
> > -		feature[4] =3D '\0';
> > -
> > =C2=A0		for (i =3D 0; i < min(lbc->num_bundles,
> > STATE_MAX_NUM_IN_BUNDLE); i++) {
> > =C2=A0			get_feature(bundle[i].encoding, feature);
> > -			printf("=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Feature %d:=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0 %s\n",
> > i, feature);
> > +			printf("=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Feature %d:=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0
> > %.4s\n", i, feature);
> > =C2=A0		}
> > =C2=A0
> > =C2=A0		if (lbc->num_bundles > STATE_MAX_NUM_IN_BUNDLE)
> >=20
>=20
> Hi,
>=20
> After staring this for a while, I cannot get rid of the feeling that the=
=20
> removal of NUL termination is a step into wrong direction. But IMO,=20
> instead of the caller side, the NUL termination could be added inside=20
> get_feature().

Yeah, you're right. I'll make this change.

David
>=20