diff mbox series

[2/3] lib: fwts_efi_module: use the new module loading helper functions

Message ID 20181108144511.12678-3-colin.king@canonical.com
State Accepted
Headers show
Series Remove modprobe, replace with module system calls | expand

Commit Message

Colin Ian King Nov. 8, 2018, 2:45 p.m. UTC 
From: Colin Ian King <colin.king@canonical.com>

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/lib/src/fwts_efi_module.c | 55 +++++------------------------------
 1 file changed, 7 insertions(+), 48 deletions(-)

Comments

Alex Hung Nov. 13, 2018, 7:03 a.m. UTC | #1 
On 2018-11-08 10:45 p.m., Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/lib/src/fwts_efi_module.c | 55 +++++------------------------------
>  1 file changed, 7 insertions(+), 48 deletions(-)
> 
> diff --git a/src/lib/src/fwts_efi_module.c b/src/lib/src/fwts_efi_module.c
> index c44f05a8..698d4bd6 100644
> --- a/src/lib/src/fwts_efi_module.c
> +++ b/src/lib/src/fwts_efi_module.c
> @@ -29,36 +29,6 @@
>  static char *efi_dev_name = NULL;
>  static char *module_name = NULL;
>  
> -/*
> - *  check_module_loaded()
> - *	check if a given module is loaded
> - */
> -static int check_module_loaded(
> -	fwts_framework *fw,
> -	char *module,
> -	bool *loaded)
> -{
> -	FILE *fp;
> -
> -	*loaded = false;
> -
> -	if ((fp = fopen("/proc/modules", "r")) != NULL) {
> -		char buffer[1024];
> -
> -		while (fgets(buffer, sizeof(buffer), fp) != NULL) {
> -			if (strstr(buffer, module)) {
> -				*loaded = true;
> -				break;
> -			}
> -		}
> -		(void)fclose(fp);
> -		return FWTS_OK;
> -	}
> -	fwts_log_error(fw, "Could not open /proc/modules to check if efi module '%s' is loaded.", module);
> -
> -	return FWTS_ERROR;
> -}
> -
>  /*
>   *  check_module_loaded_no_dev()
>   *	sanity check - we don't have a device so we definitely should
> @@ -70,7 +40,7 @@ static int check_module_loaded_no_dev(
>  {
>  	bool loaded;
>  
> -	if (check_module_loaded(fw, module, &loaded) != FWTS_OK)
> +	if (fwts_module_loaded(fw, module, &loaded) != FWTS_OK)
>  		return FWTS_ERROR;
>  	if (loaded) {
>  		fwts_log_error(fw, "Module '%s' is already loaded, but device not available.", module);
> @@ -106,16 +76,12 @@ static int load_module(
>  	char *module,
>  	char *devname)
>  {
> -	int status;
> -	char cmd[80];
>  	bool loaded;
>  
> -	snprintf(cmd, sizeof(cmd), "modprobe %s", module);
> -
> -	if (fwts_exec(cmd, &status) != FWTS_OK)
> +	if (fwts_module_load(fw, module) != FWTS_OK)
>  		return FWTS_ERROR;
>  
> -	if (check_module_loaded(fw, module, &loaded) != FWTS_OK)
> +	if (fwts_module_loaded(fw, module, &loaded) != FWTS_OK)
>  		return FWTS_ERROR;
>  
>  	if (!loaded)
> @@ -170,8 +136,7 @@ int fwts_lib_efi_runtime_load_module(fwts_framework *fw)
>  int fwts_lib_efi_runtime_unload_module(fwts_framework *fw)
>  {
>  	bool loaded;
> -	int status;
> -	char cmd[80], *tmp_name = module_name;
> +	char *tmp_name = module_name;
>  
>  	efi_dev_name = NULL;
>  
> @@ -181,20 +146,14 @@ int fwts_lib_efi_runtime_unload_module(fwts_framework *fw)
>  
>  	module_name = NULL;
>  
> -	/* If it is not loaded, no need to unload it */
> -	if (check_module_loaded(fw, tmp_name, &loaded) != FWTS_OK)
> -		return FWTS_ERROR;
> -	if (!loaded)
> -		return FWTS_OK;
> -
> -	snprintf(cmd, sizeof(cmd), "modprobe -r %s", tmp_name);
> -	if (fwts_exec(cmd, &status) != FWTS_OK) {
> +	/* Unload module */
> +	if (fwts_module_unload(fw, tmp_name) != FWTS_OK) {
>  		fwts_log_error(fw, "Failed to unload module '%s'.", tmp_name);
>  		return FWTS_ERROR;
>  	}
>  
>  	/* Module should not be loaded at this point */
> -	if (check_module_loaded(fw, tmp_name, &loaded) != FWTS_OK)
> +	if (fwts_module_loaded(fw, tmp_name, &loaded) != FWTS_OK)
>  		return FWTS_ERROR;
>  	if (loaded) {
>  		fwts_log_error(fw, "Failed to unload module '%s'.", tmp_name);
> 


Acked-by: Alex Hung <alex.hung@canonical.com>
Ivan Hu Nov. 13, 2018, 7:52 a.m. UTC | #2 
On 11/8/18 10:45 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/lib/src/fwts_efi_module.c | 55 +++++------------------------------
>  1 file changed, 7 insertions(+), 48 deletions(-)
>
> diff --git a/src/lib/src/fwts_efi_module.c b/src/lib/src/fwts_efi_module.c
> index c44f05a8..698d4bd6 100644
> --- a/src/lib/src/fwts_efi_module.c
> +++ b/src/lib/src/fwts_efi_module.c
> @@ -29,36 +29,6 @@
>  static char *efi_dev_name = NULL;
>  static char *module_name = NULL;
>  
> -/*
> - *  check_module_loaded()
> - *	check if a given module is loaded
> - */
> -static int check_module_loaded(
> -	fwts_framework *fw,
> -	char *module,
> -	bool *loaded)
> -{
> -	FILE *fp;
> -
> -	*loaded = false;
> -
> -	if ((fp = fopen("/proc/modules", "r")) != NULL) {
> -		char buffer[1024];
> -
> -		while (fgets(buffer, sizeof(buffer), fp) != NULL) {
> -			if (strstr(buffer, module)) {
> -				*loaded = true;
> -				break;
> -			}
> -		}
> -		(void)fclose(fp);
> -		return FWTS_OK;
> -	}
> -	fwts_log_error(fw, "Could not open /proc/modules to check if efi module '%s' is loaded.", module);
> -
> -	return FWTS_ERROR;
> -}
> -
>  /*
>   *  check_module_loaded_no_dev()
>   *	sanity check - we don't have a device so we definitely should
> @@ -70,7 +40,7 @@ static int check_module_loaded_no_dev(
>  {
>  	bool loaded;
>  
> -	if (check_module_loaded(fw, module, &loaded) != FWTS_OK)
> +	if (fwts_module_loaded(fw, module, &loaded) != FWTS_OK)
>  		return FWTS_ERROR;
>  	if (loaded) {
>  		fwts_log_error(fw, "Module '%s' is already loaded, but device not available.", module);
> @@ -106,16 +76,12 @@ static int load_module(
>  	char *module,
>  	char *devname)
>  {
> -	int status;
> -	char cmd[80];
>  	bool loaded;
>  
> -	snprintf(cmd, sizeof(cmd), "modprobe %s", module);
> -
> -	if (fwts_exec(cmd, &status) != FWTS_OK)
> +	if (fwts_module_load(fw, module) != FWTS_OK)
>  		return FWTS_ERROR;
>  
> -	if (check_module_loaded(fw, module, &loaded) != FWTS_OK)
> +	if (fwts_module_loaded(fw, module, &loaded) != FWTS_OK)
>  		return FWTS_ERROR;
>  
>  	if (!loaded)
> @@ -170,8 +136,7 @@ int fwts_lib_efi_runtime_load_module(fwts_framework *fw)
>  int fwts_lib_efi_runtime_unload_module(fwts_framework *fw)
>  {
>  	bool loaded;
> -	int status;
> -	char cmd[80], *tmp_name = module_name;
> +	char *tmp_name = module_name;
>  
>  	efi_dev_name = NULL;
>  
> @@ -181,20 +146,14 @@ int fwts_lib_efi_runtime_unload_module(fwts_framework *fw)
>  
>  	module_name = NULL;
>  
> -	/* If it is not loaded, no need to unload it */
> -	if (check_module_loaded(fw, tmp_name, &loaded) != FWTS_OK)
> -		return FWTS_ERROR;
> -	if (!loaded)
> -		return FWTS_OK;
> -
> -	snprintf(cmd, sizeof(cmd), "modprobe -r %s", tmp_name);
> -	if (fwts_exec(cmd, &status) != FWTS_OK) {
> +	/* Unload module */
> +	if (fwts_module_unload(fw, tmp_name) != FWTS_OK) {
>  		fwts_log_error(fw, "Failed to unload module '%s'.", tmp_name);
>  		return FWTS_ERROR;
>  	}
>  
>  	/* Module should not be loaded at this point */
> -	if (check_module_loaded(fw, tmp_name, &loaded) != FWTS_OK)
> +	if (fwts_module_loaded(fw, tmp_name, &loaded) != FWTS_OK)
>  		return FWTS_ERROR;
>  	if (loaded) {
>  		fwts_log_error(fw, "Failed to unload module '%s'.", tmp_name);


Acked-by: Ivan Hu <ivan.hu@canonical.com>
diff mbox series

Patch

diff --git a/src/lib/src/fwts_efi_module.c b/src/lib/src/fwts_efi_module.c
index c44f05a8..698d4bd6 100644
--- a/src/lib/src/fwts_efi_module.c
+++ b/src/lib/src/fwts_efi_module.c
@@ -29,36 +29,6 @@ 
 static char *efi_dev_name = NULL;
 static char *module_name = NULL;
 
-/*
- *  check_module_loaded()
- *	check if a given module is loaded
- */
-static int check_module_loaded(
-	fwts_framework *fw,
-	char *module,
-	bool *loaded)
-{
-	FILE *fp;
-
-	*loaded = false;
-
-	if ((fp = fopen("/proc/modules", "r")) != NULL) {
-		char buffer[1024];
-
-		while (fgets(buffer, sizeof(buffer), fp) != NULL) {
-			if (strstr(buffer, module)) {
-				*loaded = true;
-				break;
-			}
-		}
-		(void)fclose(fp);
-		return FWTS_OK;
-	}
-	fwts_log_error(fw, "Could not open /proc/modules to check if efi module '%s' is loaded.", module);
-
-	return FWTS_ERROR;
-}
-
 /*
  *  check_module_loaded_no_dev()
  *	sanity check - we don't have a device so we definitely should
@@ -70,7 +40,7 @@  static int check_module_loaded_no_dev(
 {
 	bool loaded;
 
-	if (check_module_loaded(fw, module, &loaded) != FWTS_OK)
+	if (fwts_module_loaded(fw, module, &loaded) != FWTS_OK)
 		return FWTS_ERROR;
 	if (loaded) {
 		fwts_log_error(fw, "Module '%s' is already loaded, but device not available.", module);
@@ -106,16 +76,12 @@  static int load_module(
 	char *module,
 	char *devname)
 {
-	int status;
-	char cmd[80];
 	bool loaded;
 
-	snprintf(cmd, sizeof(cmd), "modprobe %s", module);
-
-	if (fwts_exec(cmd, &status) != FWTS_OK)
+	if (fwts_module_load(fw, module) != FWTS_OK)
 		return FWTS_ERROR;
 
-	if (check_module_loaded(fw, module, &loaded) != FWTS_OK)
+	if (fwts_module_loaded(fw, module, &loaded) != FWTS_OK)
 		return FWTS_ERROR;
 
 	if (!loaded)
@@ -170,8 +136,7 @@  int fwts_lib_efi_runtime_load_module(fwts_framework *fw)
 int fwts_lib_efi_runtime_unload_module(fwts_framework *fw)
 {
 	bool loaded;
-	int status;
-	char cmd[80], *tmp_name = module_name;
+	char *tmp_name = module_name;
 
 	efi_dev_name = NULL;
 
@@ -181,20 +146,14 @@  int fwts_lib_efi_runtime_unload_module(fwts_framework *fw)
 
 	module_name = NULL;
 
-	/* If it is not loaded, no need to unload it */
-	if (check_module_loaded(fw, tmp_name, &loaded) != FWTS_OK)
-		return FWTS_ERROR;
-	if (!loaded)
-		return FWTS_OK;
-
-	snprintf(cmd, sizeof(cmd), "modprobe -r %s", tmp_name);
-	if (fwts_exec(cmd, &status) != FWTS_OK) {
+	/* Unload module */
+	if (fwts_module_unload(fw, tmp_name) != FWTS_OK) {
 		fwts_log_error(fw, "Failed to unload module '%s'.", tmp_name);
 		return FWTS_ERROR;
 	}
 
 	/* Module should not be loaded at this point */
-	if (check_module_loaded(fw, tmp_name, &loaded) != FWTS_OK)
+	if (fwts_module_loaded(fw, tmp_name, &loaded) != FWTS_OK)
 		return FWTS_ERROR;
 	if (loaded) {
 		fwts_log_error(fw, "Failed to unload module '%s'.", tmp_name);