diff mbox series

[v1,1/3] efi: filestore: don't compile when config disabled

Message ID 20231121235713.1530289-2-i@shantur.com
State Superseded
Delegated to: Heinrich Schuchardt
Headers show
Series efi_vars: SPI Flash store | expand

Commit Message

Shantur Rathore Nov. 21, 2023, 11:57 p.m. UTC 
Compile out filestore functions when config isn't enabled.

Signed-off-by: Shantur Rathore <i@shantur.com>
---

 include/efi_variable.h        | 21 ++++++++++++---------
 lib/efi_loader/efi_var_file.c | 13 +++++++------
 lib/efi_loader/efi_variable.c | 10 +++++++++-
 3 files changed, 28 insertions(+), 16 deletions(-)

Comments

AKASHI Takahiro Nov. 22, 2023, 2:13 a.m. UTC | #1 
On Tue, Nov 21, 2023 at 11:57:11PM +0000, Shantur Rathore wrote:
> Compile out filestore functions when config isn't enabled.
> 
> Signed-off-by: Shantur Rathore <i@shantur.com>
> ---
> 
>  include/efi_variable.h        | 21 ++++++++++++---------
>  lib/efi_loader/efi_var_file.c | 13 +++++++------
>  lib/efi_loader/efi_variable.c | 10 +++++++++-
>  3 files changed, 28 insertions(+), 16 deletions(-)
> 
> diff --git a/include/efi_variable.h b/include/efi_variable.h
> index 805e6c5f1e..ca7e19d514 100644
> --- a/include/efi_variable.h
> +++ b/include/efi_variable.h
> @@ -136,15 +136,6 @@ struct efi_var_file {
>  	struct efi_var_entry var[];
>  };
>  
> -/**
> - * efi_var_to_file() - save non-volatile variables as file
> - *
> - * File ubootefi.var is created on the EFI system partion.
> - *
> - * Return:	status code
> - */
> -efi_status_t efi_var_to_file(void);
> -
>  /**
>   * efi_var_collect() - collect variables in buffer
>   *
> @@ -172,6 +163,16 @@ efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *
>   */
>  efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe);
>  
> +#ifdef CONFIG_EFI_VARIABLE_FILE_STORE

I don't think we need this guard because any function declaration
in a header is harmless even if that function implementation is opted out.

> +/**
> + * efi_var_to_file() - save non-volatile variables as file
> + *
> + * File ubootefi.var is created on the EFI system parition.
> + *
> + * Return:	status code
> + */
> +efi_status_t efi_var_to_file(void);
> +
>  /**
>   * efi_var_from_file() - read variables from file
>   *
> @@ -185,6 +186,8 @@ efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe);
>   */
>  efi_status_t efi_var_from_file(void);
>  
> +#endif // CONFIG_EFI_VARIABLE_FILE_STORE
> +
>  /**
>   * efi_var_mem_init() - set-up variable list
>   *
> diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
> index 62e071bd83..7ceb7e3cf7 100644
> --- a/lib/efi_loader/efi_var_file.c
> +++ b/lib/efi_loader/efi_var_file.c
> @@ -117,6 +117,8 @@ efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *
>  	return EFI_SUCCESS;
>  }
>  
> +#ifdef CONFIG_EFI_VARIABLE_FILE_STORE

Can you refactor the code further to purge all "#ifdef" stuffs from this file?

I mean:
1) move efi_var_collect/restor() to a always-compiled file, say,
   efi_var_common.c/efi_variable.c
2) modify call sites of the other functions, for example, as follows:
     if (CONFIG_IS_ENABLED(EFI_VARIABLE_STORE))
         efi_var_to_file();
   # I'm not sure why the return code is ignored everywhere.
3) modify Makefile to compile efi_var_file.c only if EFI_VARIABLE_FILE_STORE
   is enabled
4) remove "#ifdef CONFIG_EFI_VARIABLE_FILE_STORE" from efi_var_file.c

-Takahiro Akashi

> +
>  /**
>   * efi_var_to_file() - save non-volatile variables as file
>   *
> @@ -126,7 +128,6 @@ efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *
>   */
>  efi_status_t efi_var_to_file(void)
>  {
> -#ifdef CONFIG_EFI_VARIABLE_FILE_STORE
>  	efi_status_t ret;
>  	struct efi_var_file *buf;
>  	loff_t len;
> @@ -150,11 +151,10 @@ error:
>  		log_err("Failed to persist EFI variables\n");
>  	free(buf);
>  	return ret;
> -#else
> -	return EFI_SUCCESS;
> -#endif
>  }
>  
> +#endif // CONFIG_EFI_VARIABLE_FILE_STORE
> +
>  efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
>  {
>  	struct efi_var_entry *var, *last_var;
> @@ -198,6 +198,7 @@ efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
>  	return EFI_SUCCESS;
>  }
>  
> +#ifdef CONFIG_EFI_VARIABLE_FILE_STORE
>  /**
>   * efi_var_from_file() - read variables from file
>   *
> @@ -211,7 +212,6 @@ efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
>   */
>  efi_status_t efi_var_from_file(void)
>  {
> -#ifdef CONFIG_EFI_VARIABLE_FILE_STORE
>  	struct efi_var_file *buf;
>  	loff_t len;
>  	efi_status_t ret;
> @@ -236,6 +236,7 @@ efi_status_t efi_var_from_file(void)
>  		log_err("Invalid EFI variables file\n");
>  error:
>  	free(buf);
> -#endif
>  	return EFI_SUCCESS;
>  }
> +
> +#endif // CONFIG_EFI_VARIABLE_FILE_STORE
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index be95ed44e6..7fa444451d 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -357,8 +357,11 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
>  	 * Write non-volatile EFI variables to file
>  	 * TODO: check if a value change has occured to avoid superfluous writes
>  	 */
> -	if (attributes & EFI_VARIABLE_NON_VOLATILE)
> +	if (attributes & EFI_VARIABLE_NON_VOLATILE) {
> +#ifdef CONFIG_EFI_VARIABLE_FILE_STORE
>  		efi_var_to_file();
> +#endif
> +	}
>  
>  	return EFI_SUCCESS;
>  }
> @@ -466,7 +469,12 @@ efi_status_t efi_init_variables(void)
>  	if (ret != EFI_SUCCESS)
>  		return ret;
>  
> +#ifdef CONFIG_EFI_VARIABLE_FILE_STORE
>  	ret = efi_var_from_file();
> +#else
> +	ret = EFI_SUCCESS;
> +#endif
> +
>  	if (ret != EFI_SUCCESS)
>  		return ret;
>  	if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {
> -- 
> 2.40.1
>
diff mbox series

Patch

diff --git a/include/efi_variable.h b/include/efi_variable.h
index 805e6c5f1e..ca7e19d514 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -136,15 +136,6 @@  struct efi_var_file {
 	struct efi_var_entry var[];
 };
 
-/**
- * efi_var_to_file() - save non-volatile variables as file
- *
- * File ubootefi.var is created on the EFI system partion.
- *
- * Return:	status code
- */
-efi_status_t efi_var_to_file(void);
-
 /**
  * efi_var_collect() - collect variables in buffer
  *
@@ -172,6 +163,16 @@  efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *
  */
 efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe);
 
+#ifdef CONFIG_EFI_VARIABLE_FILE_STORE
+/**
+ * efi_var_to_file() - save non-volatile variables as file
+ *
+ * File ubootefi.var is created on the EFI system parition.
+ *
+ * Return:	status code
+ */
+efi_status_t efi_var_to_file(void);
+
 /**
  * efi_var_from_file() - read variables from file
  *
@@ -185,6 +186,8 @@  efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe);
  */
 efi_status_t efi_var_from_file(void);
 
+#endif // CONFIG_EFI_VARIABLE_FILE_STORE
+
 /**
  * efi_var_mem_init() - set-up variable list
  *
diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
index 62e071bd83..7ceb7e3cf7 100644
--- a/lib/efi_loader/efi_var_file.c
+++ b/lib/efi_loader/efi_var_file.c
@@ -117,6 +117,8 @@  efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *
 	return EFI_SUCCESS;
 }
 
+#ifdef CONFIG_EFI_VARIABLE_FILE_STORE
+
 /**
  * efi_var_to_file() - save non-volatile variables as file
  *
@@ -126,7 +128,6 @@  efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *
  */
 efi_status_t efi_var_to_file(void)
 {
-#ifdef CONFIG_EFI_VARIABLE_FILE_STORE
 	efi_status_t ret;
 	struct efi_var_file *buf;
 	loff_t len;
@@ -150,11 +151,10 @@  error:
 		log_err("Failed to persist EFI variables\n");
 	free(buf);
 	return ret;
-#else
-	return EFI_SUCCESS;
-#endif
 }
 
+#endif // CONFIG_EFI_VARIABLE_FILE_STORE
+
 efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
 {
 	struct efi_var_entry *var, *last_var;
@@ -198,6 +198,7 @@  efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
 	return EFI_SUCCESS;
 }
 
+#ifdef CONFIG_EFI_VARIABLE_FILE_STORE
 /**
  * efi_var_from_file() - read variables from file
  *
@@ -211,7 +212,6 @@  efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
  */
 efi_status_t efi_var_from_file(void)
 {
-#ifdef CONFIG_EFI_VARIABLE_FILE_STORE
 	struct efi_var_file *buf;
 	loff_t len;
 	efi_status_t ret;
@@ -236,6 +236,7 @@  efi_status_t efi_var_from_file(void)
 		log_err("Invalid EFI variables file\n");
 error:
 	free(buf);
-#endif
 	return EFI_SUCCESS;
 }
+
+#endif // CONFIG_EFI_VARIABLE_FILE_STORE
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index be95ed44e6..7fa444451d 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -357,8 +357,11 @@  efi_status_t efi_set_variable_int(const u16 *variable_name,
 	 * Write non-volatile EFI variables to file
 	 * TODO: check if a value change has occured to avoid superfluous writes
 	 */
-	if (attributes & EFI_VARIABLE_NON_VOLATILE)
+	if (attributes & EFI_VARIABLE_NON_VOLATILE) {
+#ifdef CONFIG_EFI_VARIABLE_FILE_STORE
 		efi_var_to_file();
+#endif
+	}
 
 	return EFI_SUCCESS;
 }
@@ -466,7 +469,12 @@  efi_status_t efi_init_variables(void)
 	if (ret != EFI_SUCCESS)
 		return ret;
 
+#ifdef CONFIG_EFI_VARIABLE_FILE_STORE
 	ret = efi_var_from_file();
+#else
+	ret = EFI_SUCCESS;
+#endif
+
 	if (ret != EFI_SUCCESS)
 		return ret;
 	if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {