All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: "Wieczor-Retman, Maciej" <maciej.wieczor-retman@intel.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Reinette Chatre <reinette.chatre@intel.com>,
	fenghua.yu@intel.com
Subject: Re: [PATCH 1/3] selftests/resctrl: Fix schemata write error check
Date: Thu, 24 Aug 2023 15:52:05 +0300 (EEST)	[thread overview]
Message-ID: <5c6a2065-6771-ed87-c5ed-843f4e184ff8@linux.intel.com> (raw)
In-Reply-To: <e7ea177261ded70f0c781841b0fb67fb1eb8909d.1692880423.git.maciej.wieczor-retman@intel.com>

Ki,

You're lacking a few people from the To/Cc list. Please see KERNEL 
SELFTEST FRAMEWORK entry in MAINTAINERS.

On Thu, 24 Aug 2023, Wieczor-Retman, Maciej wrote:

> Writing bitmasks to the schemata can fail when the bitmask doesn't
> adhere to some constraints defined by what a particular CPU supports.
> Some example of constraints are max length or being having contiguous

"being having" is not good English.

> bits. The driver should properly return errors when any rule concerning
> bitmask format is broken.
> 
> Resctrl FS returns error codes from fprintf() only when fclose() is
> called.

I wonder if this is actually related to libc doing buffering between 
fprintf() and the actual write() syscall.

> Current error checking scheme allows invalid bitmasks to be
> written into schemata file and the selftest doesn't notice because the
> fclose() error code isn't checked.
> 
> Add error check to the fclose() call.
> 
> Add perror() just after fprintf so a proper error message can be seen.
> 
> Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@intel.com>
> ---
>  tools/testing/selftests/resctrl/resctrlfs.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
> index bd36ee206602..a6d0b632cbc6 100644
> --- a/tools/testing/selftests/resctrl/resctrlfs.c
> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
> @@ -532,13 +532,17 @@ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val)
>  	}
>  
>  	if (fprintf(fp, "%s\n", schema) < 0) {
> -		sprintf(reason, "Failed to write schemata in control group");
> +		sprintf(reason, "fprintf() failed with error : %s",
> +			strerror(errno));

These should use snprintf() to make sure the buffer does not overflow. 

>  		fclose(fp);
>  		ret = -1;
>  
>  		goto out;
>  	}
> -	fclose(fp);
> +	ret = fclose(fp);
> +	if (ret)
> +		sprintf(reason, "Failed to write schemata in control group : %s",
> +			strerror(errno));
>  
>  out:
>  	ksft_print_msg("Write schema \"%s\" to resctrl FS%s%s\n",
> 

-- 
 i.


  reply	other threads:[~2023-08-24 12:53 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-24 12:41 [PATCH 0/3] selftests/resctrl: Bug fix and optimizations Wieczor-Retman, Maciej
2023-08-24 12:41 ` [PATCH 1/3] selftests/resctrl: Fix schemata write error check Wieczor-Retman, Maciej
2023-08-24 12:52   ` Ilpo Järvinen [this message]
2023-08-25  6:25     ` Maciej Wieczór-Retman
2023-08-25  8:43       ` Ilpo Järvinen
2023-08-25  8:50         ` Maciej Wieczór-Retman
2023-08-24 12:41 ` [PATCH 2/3] selftests/resctrl: Move run_benchmark() to a more fitting file Wieczor-Retman, Maciej
2023-08-24 12:56   ` Ilpo Järvinen
2023-08-25  6:26     ` Maciej Wieczór-Retman
2023-08-24 12:41 ` [PATCH 3/3] selftests: Add printf attribute to ksefltest prints Wieczor-Retman, Maciej
2023-08-24 13:10   ` Ilpo Järvinen
2023-08-25  6:34     ` Maciej Wieczór-Retman
2023-08-25  8:28       ` Ilpo Järvinen
2023-08-25  9:05         ` Maciej Wieczór-Retman
2023-08-25  9:14           ` Ilpo Järvinen
2023-08-25  9:16             ` Maciej Wieczór-Retman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5c6a2065-6771-ed87-c5ed-843f4e184ff8@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=fenghua.yu@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maciej.wieczor-retman@intel.com \
    --cc=reinette.chatre@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.