Re: [Powerpc-utils-devel] [PATCH] Cleanup build warnings

[Kamalesh B. <kam...@li...>]

On 06/30/2015 10:42 PM, Nathan Fontenot wrote:
> Code cleanup to get rid of warnings generated when building powerpc-utils.
>
> For the most part this involves simple code changes. For the serv_config
> the code updates needed to get rid of the strict-aliasing warnings I thought
> to be more work than was worth the effort so I simply disabled strict-aliasing
> for this command.
>
> Signed-off-by: Nathan Fontenot <nf...@li...>
> ---
[...]

This patch fixes the lparstat.c. 'unsued' variable warning.

>  /**
> diff --git a/src/drmgr/common_cpu.c b/src/drmgr/common_cpu.c
> index e4c8082..c36f669 100644
> --- a/src/drmgr/common_cpu.c
> +++ b/src/drmgr/common_cpu.c
> @@ -194,8 +194,12 @@ cpu_index_to_path(struct dr_node *cpu)
>
>  	closedir(d);
>
> -	if (found)
> +	if (found) {
>  		snprintf(cpu->ofdt_path, DR_PATH_MAX, "%s", path);
> +		rc = 0;
> +	} else {
> +		rc = -1;
> +	}

We can avoid this else block by declaring  = -1;

>  	return rc;
>  }
[...]

> diff --git a/src/drmgr/drslot_chrp_cpu.c b/src/drmgr/drslot_chrp_cpu.c
> index 54fc576..f34854c 100644
> --- a/src/drmgr/drslot_chrp_cpu.c
> +++ b/src/drmgr/drslot_chrp_cpu.c
> @@ -352,8 +352,8 @@ int
>  drslot_chrp_cpu(struct options *opts)
>  {
>  	struct dr_info dr_info;
> -	int rc = -1;
> -
> +	int rc;
> +	

Above line introduces trailing white spaces.

>  	if (! cpu_dlpar_capable()) {
>  		say(ERROR, "CPU DLPAR capability is not enabled on this "
>  		    "platform.\n");
> @@ -386,15 +386,20 @@ drslot_chrp_cpu(struct options *opts)
>
>  	if (opts->p_option && (strcmp(opts->p_option, "smt_threads") == 0)) {
>  		rc = smt_threads_func(opts, &dr_info);
> -	} else {
> -		switch (opts->action) {
> -		    case ADD:
> -			rc = add_cpus(opts, &dr_info);
> -			break;
> -		    case REMOVE:
> -			rc = remove_cpus(opts, &dr_info);
> -			break;
> -		}
> +		free_cpu_drc_info(&dr_info);
> +		return rc;
> +	} 
> +	

Above line introduces trailing white spaces.
> +	switch (opts->action) {
> +	case ADD:
> +		rc = add_cpus(opts, &dr_info);
> +		break;
> +	case REMOVE:
> +		rc = remove_cpus(opts, &dr_info);
> +		break;
> +	default:
> +		rc = -1;
> +		break;
>  	}
>
>  	free_cpu_drc_info(&dr_info);
[...]

> diff --git a/src/serv_config.c b/src/serv_config.c
> index 46e02f3..900f0f3 100644
> --- a/src/serv_config.c
> +++ b/src/serv_config.c
> @@ -463,7 +463,7 @@ int
>  update_nvram(char *var, char *val, char *partition) {
>  	char buf[256];
>  	pid_t child;
> -	int status, rc;
> +	int status;
>  	char *nvram_args[] = { "nvram", "--update-config",
>  			buf, "-p", partition, NULL };
>
> @@ -482,12 +482,12 @@ update_nvram(char *var, char *val, char *partition) {
>  	}
>  	else if (child == 0) {
>  		/* child process */
> -		rc = execv(NVRAM_PROGRAM, nvram_args);
> -
> -		/* shouldn't get here */
> -		err_msg(ERR_MSG, "Could not exec %s to update NVRAM\n",
> +		if (execv(NVRAM_PROGRAM, nvram_args)) {
> +			/* shouldn't get here */
> +			err_msg(ERR_MSG, "Could not exec %s to update NVRAM\n",
>  				NVRAM_PROGRAM);
> -		exit(1);
> +			exit(1);
> +		}
>  	}
>  	else {
>  		/* parent process */
> @@ -609,12 +609,11 @@ byte_to_string(uint8_t num, char *buf, size_t size) {
>   */
>  int
>  parse_call_home_buffer(char *var, char *buf, size_t size) {
> -	int buf_size;
>  	char *loc;
>
>  	if (!call_home_buffer) return RC_OTHER;	/* should never happen */
>
> -	buf_size = be16toh(*(uint16_t *)call_home_buffer);
> +	/* The first 16 bits is the call home bufer size, skip past this. */

Spelling of 'buffer' is misspelled.

>  	loc = call_home_buffer + sizeof(uint16_t);
>
>  	while (loc[0] != '\0') {
>

-- 
Cheers,
Kamalesh.