Received-SPF: pass (google.com: domain of linux-kernel+bounces-58251-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Date: Thu, 8 Feb 2024 16:52:58 +0200 (EET)
To: "David E. Box" <david.e.box@linux.intel.com>
cc: Netdev <netdev@vger.kernel.org>, 
    sathyanarayanan.kuppuswamy@linux.intel.com, 
    LKML <linux-kernel@vger.kernel.org>, platform-driver-x86@vger.kernel.org
Subject: Re: [PATCH 8/8] tools: intel_sdsi: Add current meter support
In-Reply-To: <20240201010747.471141-9-david.e.box@linux.intel.com>
Message-ID: <ffdd7e28-9ed1-9380-7666-a7b108d84949@linux.intel.com>
References: <20240201010747.471141-1-david.e.box@linux.intel.com> <20240201010747.471141-9-david.e.box@linux.intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII

On Wed, 31 Jan 2024, David E. Box wrote:

> Add support to read the 'meter_current' file. The display is the same as
> the 'meter_certificate', but will show the current snapshot of the
> counters.
> 
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
>  tools/arch/x86/intel_sdsi/intel_sdsi.c | 48 +++++++++++++++++---------
>  1 file changed, 31 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/arch/x86/intel_sdsi/intel_sdsi.c b/tools/arch/x86/intel_sdsi/intel_sdsi.c
> index a8fb6d17405f..c9b3e457885d 100644
> --- a/tools/arch/x86/intel_sdsi/intel_sdsi.c
> +++ b/tools/arch/x86/intel_sdsi/intel_sdsi.c
> @@ -182,6 +182,7 @@ struct sdsi_dev {
>  enum command {
>  	CMD_SOCKET_INFO,
>  	CMD_METER_CERT,
> +	CMD_METER_CURRENT_CERT,
>  	CMD_STATE_CERT,
>  	CMD_PROV_AKC,
>  	CMD_PROV_CAP,
> @@ -329,7 +330,7 @@ static void get_feature(uint32_t encoding, char *feature)
>  	feature[0] = name[3];
>  }
>  
> -static int sdsi_meter_cert_show(struct sdsi_dev *s)
> +static int sdsi_meter_cert_show(struct sdsi_dev *s, bool show_current)
>  {
>  	char buf[METER_CERT_MAX_SIZE] = {0};
>  	struct bundle_encoding_counter *bec;
> @@ -360,7 +361,11 @@ static int sdsi_meter_cert_show(struct sdsi_dev *s)
>  		return ret;
>  	}
>  
> -	cert_ptr = fopen("meter_certificate", "r");
> +	if (!show_current)
> +		cert_ptr = fopen("meter_certificate", "r");
> +	else
> +		cert_ptr = fopen("meter_current", "r");

Please calculate the filename only once:

	cert_fname = show_current ? "meter_current" : "meter_certificate";
	cert_ptr = fopen(cert_fname, "r");

> +
>  	if (!cert_ptr) {
>  		perror("Could not open 'meter_certificate' file");

This line still has 'meter_certificate' so you need to convert it to 
fprintf() + strerror().

>  		return -1;
> @@ -368,7 +373,8 @@ static int sdsi_meter_cert_show(struct sdsi_dev *s)
>  
>  	size = fread(buf, 1, sizeof(buf), cert_ptr);
>  	if (!size) {
> -		fprintf(stderr, "Could not read 'meter_certificate' file\n");
> +		fprintf(stderr, "Could not read '%s' file\n",
> +			show_current ? "meter_current" : "meter_certificate");

Use the local variable from above.

-- 
 i.