Re: [Libguestfs] [PATCH] New API: btrfs_device_stats

>> @@ -2083,3 +2083,72 @@ do_btrfs_image (char *const *sources, const char *image, >> >> return 0; >> } >> + >> +char ** >> +do_btrfs_device_stats (const char *path, int zero) >> +{ >> + const size_t MAX_ARGS = 64; >> + const char *argv[MAX_ARGS]; >> + size_t i = 0; >> + CLEANUP_FREE char *buf = NULL; >> + CLEANUP_FREE char *err = NULL; >> + CLEANUP_FREE char *out = NULL; >> + char *p, *key = NULL, *value = NULL; >> + DECLARE_STRINGSBUF (ret); > > 'ret' is leaked if returning before "return ret.argv". > yup...will fix this. see some other APIs have the same problem.

>> + if (add_string (&ret, key) == -1) >> + return NULL; >> + >> + if (add_string (&ret, value) == -1) >> + return NULL; >> + >> + p = analyze_line(p, &key, &value, ' '); >> + } > > This means that the return "hash" will have keys like: > [/dev/sda].write_io_errs > ? Wouldn't it better to just return the name of the attribute, i.e. > write_io_errs > ? In the condition that the btrfs have multi devices, its original output is going to this way: [/dev/sda].write_io_errs 0 [/dev/sda].read_io_errs 0 [/dev/sda].flush_io_errs 0 [/dev/sda].corruption_errs 0 [/dev/sda].generation_errs 0 [/dev/sdb].write_io_errs 0 [/dev/sdb].read_io_errs 0 [/dev/sdb].flush_io_errs 0 [/dev/sdb].corruption_errs 0 [/dev/sdb].generation_errs 0 [/dev/sdc]... [/dev/sdc]... [/dev/sdc]... [/dev/sdc]... ... So. I think the [/dev/sd..] is necessary, how to think?