From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x442.google.com (mail-wr1-x442.google.com [IPv6:2a00:1450:4864:20::442]) by gabe.freedesktop.org (Postfix) with ESMTPS id 099CB6E8C1 for ; Tue, 28 Apr 2020 18:40:11 +0000 (UTC) Received: by mail-wr1-x442.google.com with SMTP id f13so25852361wrm.13 for ; Tue, 28 Apr 2020 11:40:10 -0700 (PDT) References: <20200421161725.12249-1-juhapekka.heikkila@gmail.com> <20200421161725.12249-4-juhapekka.heikkila@gmail.com> <20200428122622.GQ9497@platvala-desk.ger.corp.intel.com> From: Juha-Pekka Heikkila Message-ID: <60ba83a4-e002-792e-c383-95c471ab21bd@gmail.com> Date: Tue, 28 Apr 2020 21:40:00 +0300 MIME-Version: 1.0 In-Reply-To: <20200428122622.GQ9497@platvala-desk.ger.corp.intel.com> Content-Language: en-US Subject: Re: [igt-dev] [PATCH i-g-t 3/8] lib/params: overhaul param saving List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: juhapekka.heikkila@gmail.com Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Petri Latvala Cc: igt-dev@lists.freedesktop.org, Jani Nikula List-ID: On 28.4.2020 15.26, Petri Latvala wrote: > On Tue, Apr 21, 2020 at 07:17:20PM +0300, Juha-Pekka Heikkila wrote: >> From: Jani Nikula >> >> More generic, use existing functions. >> >> Signed-off-by: Jani Nikula >> Signed-off-by: Juha-Pekka Heikkila >> --- >> lib/igt_params.c | 101 +++++++++++++++++++---------------------------- >> 1 file changed, 41 insertions(+), 60 deletions(-) >> >> diff --git a/lib/igt_params.c b/lib/igt_params.c >> index f220e73b..b5ac1266 100644 >> --- a/lib/igt_params.c >> +++ b/lib/igt_params.c >> @@ -34,92 +34,74 @@ >> #include "igt_params.h" >> #include "igt_sysfs.h" >> >> -#define MODULE_PARAM_DIR "/sys/module/i915/parameters/" >> -#define PARAM_NAME_MAX_SZ 32 >> -#define PARAM_VALUE_MAX_SZ 16 >> -#define PARAM_FILE_PATH_MAX_SZ (strlen(MODULE_PARAM_DIR) + PARAM_NAME_MAX_SZ) >> - >> struct module_param_data { >> - char name[PARAM_NAME_MAX_SZ]; >> - char original_value[PARAM_VALUE_MAX_SZ]; >> + char *path; >> + char *name; >> + char *original_value; >> >> struct module_param_data *next; >> }; >> struct module_param_data *module_params = NULL; >> >> -static void igt_module_param_exit_handler(int sig) >> +static void igt_params_exit_handler(int sig) >> { >> - const size_t dir_len = strlen(MODULE_PARAM_DIR); >> - char file_path[PARAM_FILE_PATH_MAX_SZ]; >> struct module_param_data *data; >> - int fd; >> - >> - /* We don't need to assert string sizes on this function since they were >> - * already checked before being stored on the lists. Besides, >> - * igt_assert() is not AS-Safe. */ >> - strcpy(file_path, MODULE_PARAM_DIR); >> + int dir; >> >> for (data = module_params; data != NULL; data = data->next) { >> - strcpy(file_path + dir_len, data->name); >> - >> - fd = open(file_path, O_RDWR); >> - if (fd >= 0) { >> - int size = strlen (data->original_value); >> - >> - if (size != write(fd, data->original_value, size)) { >> - const char msg[] = "WARNING: Module parameters " >> - "may not have been reset to their " >> - "original values\n"; >> - assert(write(STDERR_FILENO, msg, sizeof(msg)) >> - == sizeof(msg)); >> - } >> - >> - close(fd); >> + dir = open(data->path, O_RDONLY); >> + >> + if (!igt_sysfs_set(dir, data->name, data->original_value)) { >> + const char msg[] = "WARNING: Module parameters " >> + "may not have been reset to their " >> + "original values\n"; >> + assert(write(STDERR_FILENO, msg, sizeof(msg)) >> + == sizeof(msg)); >> } >> + >> + close(dir); >> } >> /* free() is not AS-Safe, so we can't call it here. */ >> } >> >> /** >> - * igt_save_module_param: >> - * @name: name of the i915.ko module parameter >> - * @file_path: full sysfs file path for the parameter >> + * igt_params_save: >> + * @dir: file handle for path >> + * @path: full path to the sysfs directory >> + * @name: name of the sysfs attribute >> * >> - * Reads the current value of an i915.ko module parameter, saves it on an array, >> - * then installs an exit handler to restore it when the program exits. >> + * Reads the current value of a sysfs attribute, saves it on an array, then >> + * installs an exit handler to restore it when the program exits. >> * >> * It is safe to call this function multiple times for the same parameter. >> * >> * Notice that this function is called by igt_set_module_param(), so that one - >> * or one of its wrappers - is the only function the test programs need to call. >> */ >> -static void igt_save_module_param(const char *name, const char *file_path) >> +static void igt_params_save(int dir, const char *path, const char *name) >> { >> struct module_param_data *data; >> - size_t n; >> - int fd; >> >> /* Check if this parameter is already saved. */ >> for (data = module_params; data != NULL; data = data->next) >> - if (strncmp(data->name, name, PARAM_NAME_MAX_SZ) == 0) >> + if (strcmp(data->path, path) == 0 && >> + strcmp(data->name, name) == 0) >> return; >> >> if (!module_params) >> - igt_install_exit_handler(igt_module_param_exit_handler); >> + igt_install_exit_handler(igt_params_exit_handler); >> >> data = calloc(1, sizeof (*data)); >> igt_assert(data); >> >> - strncpy(data->name, name, PARAM_NAME_MAX_SZ - 1); >> - >> - fd = open(file_path, O_RDONLY); >> - igt_assert(fd >= 0); >> + data->path = strdup(path); >> + igt_assert(data->path); >> >> - n = read(fd, data->original_value, PARAM_VALUE_MAX_SZ); >> - igt_assert_f(n > 0 && n < PARAM_VALUE_MAX_SZ, >> - "Need to increase PARAM_VALUE_MAX_SZ\n"); >> + data->name = strdup(name); >> + igt_assert(data->name); >> >> - igt_assert(close(fd) == 0); >> + data->original_value = igt_sysfs_get(dir, name); >> + igt_assert(data->original_value); >> >> data->next = module_params; >> module_params = data; >> @@ -205,22 +187,21 @@ bool igt_params_set(int device, const char *parameter, const char *fmt, ...) >> */ >> void igt_set_module_param(const char *name, const char *val) >> { >> - char file_path[PARAM_FILE_PATH_MAX_SZ]; >> - size_t len = strlen(val); >> - int fd; >> + const char *path = "/sys/module/i915/parameters"; >> + int dir; >> >> - igt_assert_f(strlen(name) < PARAM_NAME_MAX_SZ, >> - "Need to increase PARAM_NAME_MAX_SZ\n"); >> - strcpy(file_path, MODULE_PARAM_DIR); >> - strcpy(file_path + strlen(MODULE_PARAM_DIR), name); >> + dir = open(path, O_RDONLY); >> + igt_assert(dir >= 0); >> >> - igt_save_module_param(name, file_path); >> + igt_params_save(dir, path, name); >> >> - fd = open(file_path, O_RDWR); >> - igt_assert(write(fd, val, len) == len); >> - igt_assert(close(fd) == 0); >> + igt_assert(igt_sysfs_set(dir, name, val)); >> + >> + igt_assert(close(dir) == 0); >> } >> >> +#define PARAM_VALUE_MAX_SZ 16 >> + > > Is this define still needed for something? Look like it has moved inside igt_aux.c so it's not needed here, I'll take it away. _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev