On Tue, Nov 01, 2016 at 04:01:44PM +0800, Qu Wenruo wrote:
> +/*
> + * Underlying PRINT_DUMP, the only difference is how we handle
> + * the full path.
> + */
> +static int __print_dump(int subvol, void *user, const char *path,
> +                     const char *title, const char *fmt, ...)

printf-like the __attribute__ ((format (printf, ...)))

> +{
> +     struct btrfs_dump_send_args *r = user;
> +     char real_title[TITLE_WIDTH + 1] = { 0 };
> +     char full_path[PATH_MAX] = {0};
> +     char *out_path;
> +     va_list args;
> +     int ret;
> +
> +     if (subvol) {
> +             PATH_CAT_OR_RET(title, r->full_subvol_path, r->root_path, path, 
> ret);
> +             out_path = r->full_subvol_path;
> +     } else {
> +             PATH_CAT_OR_RET(title, full_path, r->full_subvol_path, path, 
> ret);
> +             out_path = full_path;
> +     }
> +     string_escape_inplace(out_path, " \n\t\\");
> +
> +     /* Append ':' to title */
> +     strncpy(real_title, title, TITLE_WIDTH - 1);
> +     strncat(real_title, ":", TITLE_WIDTH);

I'd rather avoid such string operations, ':', just print everything.

> +
> +     /* Unified header, */
> +     printf("%-*s%-*s", TITLE_WIDTH, real_title, PATH_WIDTH, out_path);

PATH_WIDTH is used only here, please hardcode it into the format string.

The rest of the patch looks good. I think I've seen some artifacts in
the output, but we can tune this later.

> +     ret = strftime(dest, max_size, "%Y-%m-%d %H:%M:%S", tm);

We should use the RFC 3339 format, as it's standardized and also is a
string without whitespace.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to