btrfs-progs: send-dump: always print a space after path
PATCH v2 1/1 btrfs-progs: send-dump: always print a space after path
I was shocked to discover that 'btrfs receive --dump' doesn't print a space after long filenames, so it runs together into the metadata; for example:
truncate ./20-00-03/this-name-is-32-characters-longsize=0
This is a trivial patch to add a single space unconditionally, so the result is the following:
truncate ./20-00-03/this-name-is-32-characters-long size=0
I suppose this is technically a breaking change, but it seems unlikely to me that anyone would depend on the existing behavior given how unfriendly it is.
---
send-dump.c +3 -2
@@ -116,9 +116,10 @@ static int __print_dump(int subvol, void *user, const char *path,putchar ('\n' );return 0 ; } -/* Short paths ale aligned to 32 chars */ -while (ret++ <32 ) +/* Short paths are aligned to 32 chars; longer paths get a single space */ +do {putchar (' ' ); + }while (++ret <32 );va_start (args, fmt);/* Operation specified ones */ vprintf (fmt, args);
---
Thanks to Noah Massey for catching an off-by-one; I had kept the condition as (ret++ < 32), but that would pad to 33 spaces, not 32. Changing the loop condition to preincrement keeps the old behavior for short names.
Evan Danaher posted on Tue, 11 Apr 2017 12:33:40 -0400 as excerpted:
Quoted 9 lines
I was shocked to discover that 'btrfs receive --dump' doesn't print a space after long filenames, so it runs together into the metadata; for example:
truncate ./20-00-03/this-name-is-32-characters-longsize=0
This is a trivial patch to add a single space unconditionally, so the result is the following:
Quoted 8 lines
truncate ./20-00-03/this-name-is-32-characters-long size=0
I suppose this is technically a breaking change, but it seems unlikely to me that anyone would depend on the existing behavior given how unfriendly it is.
Signed-off-by Evan Danaher <[email protected]>---
I'm not a dev so won't attempt to comment on the patch itself, but it's worth noting that according to kernel patch submission guidelines (which btrfs-progs use as well) on V2+ patch postings, there should be a short, often one-line per version, summary of what changed between versions. This helps both reviewers and would-be patch-using admins such as myself understand how a patch is evolving, as well as for reviewers preventing unnecessary work when re-reviewing a new version of a patch previously reviewed in an earlier version.
On patch series this summary is generally found in the 0/N post, while on individual patches without a 0/N, it's normally found below the first --- delimiter, so as to avoid including the patch history in the final merged version comment.
See pretty much any other multi-version posted patch for examples.
Signature · 3 lines
Duncan - List replies preferred. No HTML msgs. "Every nonfree program has a lord, a master -- and if you use the program, he is your master." Richard Stallman
Quoted 29 lines · Duncan
Evan Danaher posted on Tue, 11 Apr 2017 12:33:40 -0400 as excerpted:
Quoted 9 lines
I was shocked to discover that 'btrfs receive --dump' doesn't print a space after long filenames, so it runs together into the metadata; for example:
truncate ./20-00-03/this-name-is-32-characters-longsize=0
This is a trivial patch to add a single space unconditionally, so the result is the following:
Quoted 8 lines
truncate ./20-00-03/this-name-is-32-characters-long size=0
I suppose this is technically a breaking change, but it seems unlikely to me that anyone would depend on the existing behavior given how unfriendly it is.
Signed-off-by Evan Danaher <[email protected]>---
I'm not a dev so won't attempt to comment on the patch itself, but it's worth noting that according to kernel patch submission guidelines (which btrfs-progs use as well) on V2+ patch postings, there should be a short, often one-line per version, summary of what changed between versions. This helps both reviewers and would-be patch-using admins such as myself understand how a patch is evolving, as well as for reviewers preventing unnecessary work when re-reviewing a new version of a patch previously reviewed in an earlier version.
Quoted 4 lines
On patch series this summary is generally found in the 0/N post, while on individual patches without a 0/N, it's normally found below the first --- delimiter, so as to avoid including the patch history in the final merged version comment.
To be specific, something like
Quoted 4 lines
---
v2: fixed an off-by-one error which caused padding to be 33 characters for short paths
---
instead of the email tail you currently appended (for future reference). FWIW, I'm fine with you adding my 'Reviewed-by', but I don't think it carries much weight yet. :-)
--
Noah
Quoted 36 lines · Noah Massey
Quoted 29 lines · Duncan
Evan Danaher posted on Tue, 11 Apr 2017 12:33:40 -0400 as excerpted:
Quoted 9 lines
I was shocked to discover that 'btrfs receive --dump' doesn't print a space after long filenames, so it runs together into the metadata; for example:
truncate ./20-00-03/this-name-is-32-characters-longsize=0
This is a trivial patch to add a single space unconditionally, so the result is the following:
Quoted 8 lines
truncate ./20-00-03/this-name-is-32-characters-long size=0
I suppose this is technically a breaking change, but it seems unlikely to me that anyone would depend on the existing behavior given how unfriendly it is.
Signed-off-by Evan Danaher <[email protected]>---
I'm not a dev so won't attempt to comment on the patch itself, but it's worth noting that according to kernel patch submission guidelines (which btrfs-progs use as well) on V2+ patch postings, there should be a short, often one-line per version, summary of what changed between versions. This helps both reviewers and would-be patch-using admins such as myself understand how a patch is evolving, as well as for reviewers preventing unnecessary work when re-reviewing a new version of a patch previously reviewed in an earlier version.
Quoted 4 lines
On patch series this summary is generally found in the 0/N post, while on individual patches without a 0/N, it's normally found below the first --- delimiter, so as to avoid including the patch history in the final merged version comment.
To be specific, something like
Quoted 8 lines
Quoted 4 lines
---
v2: fixed an off-by-one error which caused padding to be 33 characters for short paths
---instead of the email tail you currently appended (for future reference). FWIW, I'm fine with you adding my 'Reviewed-by', but I don't think it carries much weight yet. :-)
It does, I'll add the tag, thanks.
David Sterba
AppliedQuoted 9 lines · Evan Danaher
I was shocked to discover that 'btrfs receive --dump' doesn't print a space after long filenames, so it runs together into the metadata; for example:
truncate ./20-00-03/this-name-is-32-characters-longsize=0
This is a trivial patch to add a single space unconditionally, so the result is the following:
Quoted 7 lines
truncate ./20-00-03/this-name-is-32-characters-long size=0
I suppose this is technically a breaking change, but it seems unlikely to me that anyone would depend on the existing behavior given how unfriendly it is.
Signed-off-by Evan Danaher <[email protected]>
Applied, thanks.