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
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to