Received-SPF: pass (google.com: domain of linux-kernel+bounces-46387-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Wed, 31 Jan 2024 12:57:00 +0100
From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
To: Reinette Chatre <reinette.chatre@intel.com>
CC: Ilpo =?utf-8?B?SsOkcnZpbmVu?= <ilpo.jarvinen@linux.intel.com>,
	<fenghua.yu@intel.com>, <shuah@kernel.org>, LKML
	<linux-kernel@vger.kernel.org>, <linux-kselftest@vger.kernel.org>
Subject: Re: [PATCH v3 2/5] selftests/resctrl: Add helpers for the
 non-contiguous test
Message-ID: <tck7kcpjdch3a3qkkwusft5zwptlhtmicx53zjcqk3x42w5p55@pf6vrmnh7deo>
References: <cover.1706180726.git.maciej.wieczor-retman@intel.com>
 <85b1efc3ddd698b3ac81aa72a6dc987ee17da3e2.1706180726.git.maciej.wieczor-retman@intel.com>
 <ea9b0e06-c07e-eb4b-9e7d-ed20419b677d@linux.intel.com>
 <611fd2b6-d9ec-42f0-9711-b1398fc02842@intel.com>
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <611fd2b6-d9ec-42f0-9711-b1398fc02842@intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?ISKLn27WmsjGBaGHRWhqs2nFLRTGy/mFrV9QCedOaL6Yh/k7Qv/hsbIlqD?=
 =?iso-8859-1?Q?ean/55oGhh3MYN89wpiU6RO8cGNCWfd5W/bSaWIrYfkPNDLIFGHPMdwXV/?=
 =?iso-8859-1?Q?13rwEs/UNIftNhkxWuJ40M913vOtF7neyCwJyrTktGBVIp+UoW/ZtbFWCi?=
 =?iso-8859-1?Q?n2aGy1LzdLKf/OLAxJz0zJ226Oxy+ltBVao43p7RJHiEs8OIYIoga4PFMP?=
 =?iso-8859-1?Q?QlAYanmGKUmJRtxKCXdz0ZPEyW0vIn4eTmzSVST/HnuhcFyV/r96B5AcgK?=
 =?iso-8859-1?Q?HR1lHsll/M+kOdq2B0yJ30b/OzXkWlsesPsYRGltoC22Vds6XUdT2tpIfY?=
 =?iso-8859-1?Q?hODow4R/Qbf51ub7TWAV465NwI5w9Q/xq20K7lMu0rZP71VOSyMWRnffS9?=
 =?iso-8859-1?Q?8USlhqfQrCvPj+KtOsXUbxK8t6IsI6ffc6X7zS2vDz0qpHcvtLftxU6b60?=
 =?iso-8859-1?Q?5SgE7297Nng/pSH5d7eJNoc0XU1SH5Im7OrHzNC8VvJtFU5VsxMCLRgCsV?=
 =?iso-8859-1?Q?ftpFly7LPbc+z6ZebfSJJEBI1oD7/BgO04piHuUqckKUOj/KJjRklnuQZv?=
 =?iso-8859-1?Q?ivOmdQmH/i6WOB0c6Js7BAOIMbVZ5S4Mejd1u/9ZCeXIlvY6Y++AAAZSNJ?=
 =?iso-8859-1?Q?+NFNuSeDSugqgLiBtbYyJvQeZ7l2C+jeU8Cgwr0I4iVkpgoxCbM9NMkzEm?=
 =?iso-8859-1?Q?f2wlLEC9snlNgb15ayXBzIpDXlgbsoAkeItHo48fBlQdgPCbCiMSrR4PAS?=
 =?iso-8859-1?Q?L76Om6mlSuZyCHjrAqoz3fFv6eqtX/Ey5NY+KljsudmqDb+oF9lhPlIDoc?=
 =?iso-8859-1?Q?dnf2Aecl7yrcykBQL9L2ldSQoPZYlicPpWG015pZNoC8Ix9dbl5lB4A4DI?=
 =?iso-8859-1?Q?KooTY7Q7PG7yHOQrw/rnZI2QKI7+hIuMWw8UYgYOxV6I3DZx3wLbTsPqsg?=
 =?iso-8859-1?Q?cuMEbFSg//tjDagkH2/juUQxF5BN1ZxroKL8sv88edPPhhHNvNPYEn/kCD?=
 =?iso-8859-1?Q?p8B+yS4WKo3648uFw10WMYuITlgucLJlTQh+FBHSADFQkaf/dBDvFbB7Zy?=
 =?iso-8859-1?Q?GoXtIU/eFxGMoprRJQYDbbm7MTCgzlqu1QC1Zm7m4yjMNbZqRsY/cvFaAF?=
 =?iso-8859-1?Q?zezUc3Oj+a28M/uBYFXemKfPWxdUOzhRMfJv7/sNREl/bdYWw2Orsz1G0O?=
 =?iso-8859-1?Q?wQRB8F4V43w8Zoe4/jNf5MObEQ3B4fKwLvW43FK2OYSjxF9EGYeRfrIQ4F?=
 =?iso-8859-1?Q?ojp/d6+XDBUaoqwWUUKcUW7XC7BxE47Kgyrw6Pg53ASwtk+eJRW3nlorEl?=
 =?iso-8859-1?Q?OvIpkkDcOEhwXaPTnknWxiTTKjO8CH9BHyizFYDbtdp5bybDiW6AsnyymV?=
 =?iso-8859-1?Q?BfR6VoqIf5XotgScfJF+S21UlxEx7Ut1v3pp54We3huS/VozHbbQgf2kuy?=
 =?iso-8859-1?Q?Ot8AGZ5jtsS8wt7fY1ttUoDFDVUexzu5igABGiPQkSSn9STK07IHNTO+iS?=
 =?iso-8859-1?Q?8kSbYelKX+TQ7f5S9zO8vsqnC8qcgkzWN1MbqLx8SpTa1AB916lAWDflB+?=
 =?iso-8859-1?Q?IVI6YlsTwpj/FxpKJ4mKU9ODQh1einsusRiOqEVY7GeJR0qfnCkbFSkElM?=
 =?iso-8859-1?Q?kmA5vGIEWhFekYcc4r7FIvwMoPu9KEsA0dm8fh7dNclMmRWdzuS0luyM1V?=
 =?iso-8859-1?Q?40NdK46c1ztKVSb9Jt8=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 10ea813e-1acd-4e16-bc58-08dc2253bfef
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6231.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Jan 2024 11:57:07.9339
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: EeVcjwDbTOEZ/l11vreeyHufrTrFe8cPvepyYC22mTqp09ggCqgsstQwQ2UX+T8Y1PcdX0CkTw6kWPUthtz1iOJpHhxnUxRT2yYdjx5ELj8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB7064
X-OriginatorOrg: intel.com

Hi,

On 2024-01-26 at 10:58:04 -0800, Reinette Chatre wrote:
>
>
>On 1/25/2024 4:14 AM, Ilpo J?rvinen wrote:
>> On Thu, 25 Jan 2024, Maciej Wieczor-Retman wrote:
>
>
>>> +	fp = fopen(file_path, "r");
>>> +	if (!fp) {
>>> +		snprintf(reason, sizeof(reason), "Error in opening %s file\n", filename);
>>> +		ksft_perror(reason);
>> 
>> Was this the conclusion of the kstf_perror() discussion with Reinette? I 
>> expected a bit different outcome when I stopped following it...
>> 
>> In any case, it would be nice though if ksft_perror() (or some kselftest.h 
>> function yet to be added with a different name) would accept full printf 
>> interface and just add the errno string into the end of the string so one 
>> would not need to build constructs like this at all.
>> 
>> It will require a bit of macro trickery into kselftest.h. I don't know how 
>> it should handle the case where somebody just passes a char pointer to it, 
>> not a string literal, but I guess it would just throw an error while 
>> compiling if somebody tries to do that as the macro string literal 
>> concatenation could not build useful/compilable token.
>> 
>> It would make these prints informative enough to become actually useful 
>> without needed to resort to preparing the string in advance which seems
>> to be required almost every single case with the current interface.
>
>I think this can be accomplished with a new:
>	void  ksft_vprint_msg(const char *msg, va_list args)
>
>... but ksft_perror() does conform to perror() and I expect that having one
>support variable number of arguments while the other does to cause confusion.
>
>To support variable number of arguments with errno I'd propose just to use
>ksft_print_msg() with strerror(errno), errno as the arguments (or even %m
>that that errno handling within ksft_print_msg() aims to support). This does
>indeed seem to be the custom in other tests.

Does something like this look okay?

	fp = fopen(file_path, "r");
	if (!fp) {
		ksft_print_msg("Error in opening %s\n: %m\n", file_path);
		return -1;
	}

The '%m' seems to work fine but doesn't print errno's number code. Do you want
me to add errno after '%m' so it is the same as ksft_perror()? I looked through
some other tests where '%m' is used, and only few ones add errno with '%d'.

>Reinette

-- 
Kind regards
Maciej Wiecz?r-Retman