[Skiboot] [PATCH v4 03/18] secvar_main: increase error verbosity, restyle all comments

[Oliver O'Halloran]

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");
>