[Devel] [PATCH 10/10] get_num_cpu(): refactored

[Andrew Vagin]

On Tue, May 07, 2013 at 02:42:12PM +0800, Igor Podlesny wrote:
> 1) Macro MAX_OF(a, b) introduced;
> 2) FILE *fd init when declaring;
> 3) When comparing variable to const, put const first, then variable
> 	-- to reduce typo risk of unintended assignment;
> 4) Buffer moved into closer scope.
> ---
>  include/util.h |    2 ++
>  src/lib/util.c |   22 +++++++++++-----------
>  2 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/include/util.h b/include/util.h
> index 0f87e6e..49c0a83 100644
> --- a/include/util.h
> +++ b/include/util.h
> @@ -30,6 +30,8 @@
>  #define PROCTHR		"/proc/sys/kernel/threads-max"
>  #define PROCVEINFO	"/proc/vz/veinfo"
>  
> +#define MAX_OF(a, b)	( ((a) > (b)) ? (a) : (b) )
> +
>  char *parse_line(char *str, char *ltoken, int lsz, char **errstr);
>  int stat_file(const char *file);
>  int dir_empty(const char *dir);
> diff --git a/src/lib/util.c b/src/lib/util.c
> index 83cd796..2e5de1d 100644
> --- a/src/lib/util.c
> +++ b/src/lib/util.c
> @@ -538,21 +538,21 @@ int get_swap(unsigned long long *swap)
>  
>  int get_num_cpu(void)
>  {
> -	FILE *fd;
> -	char str[128];
>  	int ncpu = 0;
> +	FILE *fd = fopen(PROCCPU, "r");

Usually if a function can return an error, it does not used in
declarations.

>  
> -	if ((fd = fopen(PROCCPU, "r")) == NULL)	{
> +	if (NULL == fd)	{

Pls, follow the current coding style.
	if (fd === NULL)

>  		logger(-1, errno, "Cannot open " PROCCPU);
> -		return 1;
> -	}
> -	while (fgets(str, sizeof(str), fd)) {
> -		char const proc_ptrn[] = "processor";
> -		if (!strncmp(str, proc_ptrn, sizeof(proc_ptrn) - 1))
> -			ncpu++;
> +	} else {

Why do we need one more indent here instead of returning 1 in the
previous block? I am not sure that this version is more readable...

> +		char str[128];
> +		while (fgets(str, sizeof(str), fd)) {
> +			char const proc_ptrn[] = "processor";
> +			if (!strncmp(str, proc_ptrn, sizeof(proc_ptrn) - 1))
> +				ncpu++;
> +		}
> +		fclose(fd);
>  	}
> -	fclose(fd);
> -	return !ncpu ? 1 : ncpu;
> +	return MAX_OF(1, ncpu);
>  }
>  
>  int get_lowmem(unsigned long long *mem)
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Devel mailing list
> Devel at openvz.org
> https://lists.openvz.org/mailman/listinfo/devel