Re: [dpdk-dev] [PATCH] eal: fix check number of bytes from read function

[Thomas Monjalon <thomas.monjalon@6wind.com>]

Hi,

2016-07-20 16:24, Michal Jastrzebski:
> -	if (read(fd, &page, sizeof(uint64_t)) < 0) {
> +
> +	retval = read(fd, &page, sizeof(uint64_t));
> +	if (retval < 0) {
>  		RTE_LOG(ERR, EAL, "%s(): cannot read /proc/self/pagemap: %s\n",
>  				__func__, strerror(errno));
>  		close(fd);
>  		return RTE_BAD_PHYS_ADDR;
> +	}	else if (retval >= 0 && retval < (int)sizeof(uint64_t))	{

I have 4 comments about the above line:
- the check retval >= 0 is not needed because implied by else
- why not checking retval != sizeof(uint64_t) as it is the exact expected value?
- (int)sizeof(uint64_t) can be replaced by 8 but it's shorter ;)
- only 1 space is required between } and else

> +		RTE_LOG(ERR, EAL, "%s(): read %d bytes from /proc/self/pagemap "
> +				"but expected %d: %s\n",
> +				__func__, retval, (int)sizeof(uint64_t), strerror(errno));

Are you sure errno is meaningful here?

> +		close(fd);
> +		return RTE_BAD_PHYS_ADDR;
>  	}