[Skiboot] [PATCH v4 03/18] secvar_main: increase error verbosity, restyle all comments
Oliver O'Halloran
oohall at gmail.com
Tue May 12 13:58:34 AEST 2020
On Mon, 2020-05-11 at 16:31 -0500, Eric Richter wrote:
> This commit converts all the "//" style comments to the appropriate
> C-style, adds a few comments, and also adds a couple error log outputs.
>
> The following commits performs a minor refactor of the secvar_main
> flow. This commit has been separated out to reduce the amount of noise
> in those commits, and can likely be squashed if necessary.
Try not to mix unrelated changes. It's an 18 patch series, having 19
won't hurt anyone.
> Signed-off-by: Eric Richter <erichte at linux.ibm.com>
> ---
> V4:
> - adjusted lines to 80 columns
>
> libstb/secvar/secvar_main.c | 41 ++++++++++++++++++++++++++-----------
> 1 file changed, 29 insertions(+), 12 deletions(-)
>
> diff --git a/libstb/secvar/secvar_main.c b/libstb/secvar/secvar_main.c
> index b40dd646..3b842d16 100644
> --- a/libstb/secvar/secvar_main.c
> +++ b/libstb/secvar/secvar_main.c
> @@ -14,10 +14,10 @@
> struct list_head variable_bank;
> struct list_head update_bank;
>
> -int secvar_enabled = 0; // Set to 1 if secvar is supported
> -int secvar_ready = 0; // Set to 1 when base secvar inits correctly
> +int secvar_enabled = 0; /* Set to 1 if secvar is supported */
> +int secvar_ready = 0; /* Set to 1 when base secvar inits correctly */
>
> -// To be filled in by platform.secvar_init
> +/* To be filled in by platform.secvar_init */
> struct secvar_storage_driver secvar_storage = {0};
> struct secvar_backend_driver secvar_backend = {0};
>
> @@ -43,7 +43,7 @@ int secvar_main(struct secvar_storage_driver storage_driver,
> if (rc)
> goto fail;
>
> - // Failures here should indicate some kind of hardware problem
> + /* Failures here should indicate some kind of hardware problem */
> rc = secvar_storage.load_bank(&variable_bank, SECVAR_VARIABLE_BANK);
> if (rc)
> goto fail;
> @@ -52,21 +52,35 @@ int secvar_main(struct secvar_storage_driver storage_driver,
> if (rc)
> goto fail;
>
> - // At this point, base secvar is functional. Rest is up to the backend
> + /* At this point, base secvar is functional.
> + * The rest is up to the backend */
skiboot style for multi-line comments is:
/*
* Like this.
*
* It's the same style that most of the kernel uses.
*/
> secvar_ready = 1;
> secvar_set_status("okay");
>
> - if (secvar_backend.pre_process)
> + if (secvar_backend.pre_process) {
> rc = secvar_backend.pre_process();
> + if (rc) {
> + prlog(PR_ERR, "Error in backend pre_process = %d\n", rc);
> + goto out;
> + }
> + }
>
> - // Process is required, error if it doesn't exist
> + /* Process is required, error if it doesn't exist */
> if (!secvar_backend.process)
> goto out;
>
> + /* Process variable updates from the update bank. */
> rc = secvar_backend.process();
> - secvar_set_update_status(rc);
> +
> + /* Create and set the update-status device tree property */
> + secvar_set_update_status(rc);
> +
> + /* Only write to the storage if we actually processed updates
> + * OPAL_EMPTY implies no updates were processed
> + * Refer to full table in doc/device-tree/ibm,opal/secvar.rst */
> if (rc == OPAL_SUCCESS) {
> - rc = secvar_storage.write_bank(&variable_bank, SECVAR_VARIABLE_BANK);
> + rc = secvar_storage.write_bank(&variable_bank,
> + SECVAR_VARIABLE_BANK);
> if (rc)
> goto out;
>
> @@ -75,10 +89,13 @@ int secvar_main(struct secvar_storage_driver storage_driver,
> goto out;
> }
>
> - if (secvar_backend.post_process)
> + if (secvar_backend.post_process) {
> rc = secvar_backend.post_process();
> - if (rc)
> - goto out;
> + if (rc) {
> + prlog(PR_ERR, "Error in backend post_process = %d\n", rc);
> + goto out;
> + }
> + }
>
> prlog(PR_INFO, "secvar initialized successfully\n");
>
More information about the Skiboot
mailing list