Re: [PATCH 18/18] [RFC] user-cr: Cleanup __filepos

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 03/17/2011 01:27 PM, Matt Helsley wrote:
> Based on the original commit it's unclear why we didn't go with the far
> simpler approach of using the lseek() system call (or suitably-sized variant)
> to determine the position in the stream where the object was found. This

... because it may be an arbitrary stream, including pipe/socket which
do not support lseek() operation.

> avoids having to spread increments to this global variable into any function
> that might accidentally read the checkpoint image stream and it avoids any
> bugs dealing with error detection, retries, and return values.

That is the reason that __filepos is updated only in a single place -
__image_read() used by all other reads from the image, and only when
the operation succeeded.

I'm not sure what sort of bugs you envision (so to speak) concerning
error detection, retries and return values ?

Oren.


> 
> Signed-off-by: Matt Helsley <matthltc@xxxxxxxxxx>
> ---
>  ckptinfo.c |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/ckptinfo.c b/ckptinfo.c
> index e7609c0..5eed5ca 100644
> --- a/ckptinfo.c
> +++ b/ckptinfo.c
> @@ -41,7 +41,11 @@ struct args {
>  };
>  
>  int __verbose;
> -unsigned long __filepos;
> +
> +static inline loff_t __filepos(int fd)
> +{
> +	return lseek(fd, 0, SEEK_CUR);
> +}
>  
>  #define VERBOSE(_format, _args...)			\
>  	do {						\
> @@ -182,7 +186,6 @@ static int __image_read(int fd, void *buf, int len)
>  		return -1;
>  	}
>  
> -	__filepos += len;
>  	return len;
>  }
>  
> @@ -195,8 +198,8 @@ static int image_read_obj(int fd, struct ckpt_hdr **hh)
>  	if (ret <= 0)
>  		return ret;
>  
> -	VERBOSE("info: [@%lu] object %3d %s len %d\n",
> -		__filepos, h.type, hdr_to_str(h.type), h.len);
> +	VERBOSE("info: [@%llu] object %3d %s len %d\n",
> +		__filepos(fd), h.type, hdr_to_str(h.type), h.len);
>  
>  	p = malloc(h.len);
>  	if (!p) {
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers


[Index of Archives]     [Cgroups]     [Netdev]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux