Re: [PATCH V2 1/2] Btrfs-progs: make pretty_sizes() work less error prone

[David Sterba <dsterba@suse.cz>]

On Tue, Jul 09, 2013 at 01:24:43PM -0700, Zach Brown wrote:
> > The original codes don't handle error gracefully and some places
> > forget to free memory. We can allocate memory before calling pretty_sizes(),
> > for example, we can use static memory allocation and we don't have to deal
> > with memory allocation fails.
> 
> I agree that callers shouldn't have to know to free allocated memory.
> 
> But I think that we can do better and not have callers need to worry
> about per-call string storage at all.
> 
> How about something like this?

Neat trick! A few neat-picks below. Besides, I guess we can use this
sort of trick with the fi-df patches.

> --- a/utils.c
> +++ b/utils.c
> @@ -1153,12 +1153,13 @@ out:
>  
>  static char *size_strs[] = { "", "KB", "MB", "GB", "TB",
>  			    "PB", "EB", "ZB", "YB"};

I'll drop the ZB, YB suffixes.

> --- a/utils.h
> +++ b/utils.h
> @@ -44,7 +44,15 @@ int check_mounted_where(int fd, const char *file, char *where, int size,
>  			struct btrfs_fs_devices **fs_devices_mnt);
>  int btrfs_device_already_in_root(struct btrfs_root *root, int fd,
>  				 int super_offset);
> -char *pretty_sizes(u64 size);
> +
> +void pretty_size_snprintf(u64 size, char *str, size_t str_bytes);
> +#define pretty_sizes(size) 					\

and rename it to pretty_size as it takes only one number

> +	({							\
> +		static __thread char _str[16];			\

16 is not enough for exabyte scale, that needs at least 20 bytes + 1 for 0.

len(str(2**64)) = 20

-> 24

> +		pretty_size_snprintf(size, _str, sizeof(_str));	\

		pretty_size_snprintf((size), _str, sizeof(_str));	\

As these are only trivial changes I'll fix them at commit time.

> +		_str;						\
> +	})
> +
>  int get_mountpt(char *dev, char *mntpt, size_t size);
>  int btrfs_scan_block_devices(int run_ioctl);
>  u64 parse_size(char *s);
> -- 
> 1.7.11.7
>