btrfs-progs: send-dump: always print a space after path

PATCH Superseded Evan Danaher 3 participants 4 messages Apr 11, 2017 – Apr 19, 2017 Btrfs file system

Share & tools

Noah Massey

2017-04-11 13:25 UTC #1 lore raw

Quoted 23 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:
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]>
---
send-dump.c +0 -0
index 67f7977..493389f 100644

send-dump.c static int __print_dump(int subvol, void *user, const char *path, · line 122

Quoted 9 lines

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);

while (++ret < 32);

Since we're performing the check after the put, we need to pre-increment to count the space already added.

Quoted 8 lines

send-dump.c +0 -2
        va_start(args, fmt);
        /* Operation specified ones */
        vprintf(fmt, args);
---
--
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

Evan Danaher

author

2017-04-11 13:49 UTC #2 lore raw

Thanks for catching that! I overthought this and managed to convince myself that it was correct as it stood.

Should I re-send the whole patch with that change? This is my first attempt at contributing to a project managed in the Linux kernel style, so I'm not sure about the process; documentation that I've found around responding to feedback seems to be more about social issues than the mechanics of what to actually do...

Quoted 47 lines · Noah Massey

Quoted 23 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:
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]>
---
send-dump.c +0 -0
index 67f7977..493389f 100644
send-dump.c static int __print_dump(int subvol, void *user, const char *path, · line 122
Quoted 9 lines
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);
while (++ret < 32);
Since we're performing the check after the put, we need to pre-increment to count the space already added.
Quoted 8 lines
send-dump.c +0 -2
        va_start(args, fmt);
        /* Operation specified ones */
        vprintf(fmt, args);
---
--
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

David Sterba

2017-04-19 17:23 UTC #3 lore raw

Quoted 8 lines · Evan Danaher

Thanks for catching that! I overthought this and managed to convince myself that it was correct as it stood.
Should I re-send the whole patch with that change? This is my first attempt at contributing to a project managed in the Linux kernel style, so I'm not sure about the process; documentation that I've found around responding to feedback seems to be more about social issues than the mechanics of what to actually do...

Here's some howto
https://btrfs.wiki.kernel.org/index.php/Developer%27s_FAQ

Depends on what you take from the 'linux kernel style'. We stick to good changelogs, signed-off-by lines and patches that do one thing at a time. The interactions experience differs across the subsystems, the btrfs community is friendly.

Patches from strangers are apprecated as they usually do the extra step and also try to fix the problem they encounter. The patch does not need to be perfect, small things I fix myself. If it's not obvious it's better to do another iteration, eg. when the change needs to be retested.

Quoted 8 lines · Noah Massey

Quoted 7 lines · Evan Danaher
Quoted 6 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

Quoted 8 lines

Quoted 8 lines
Quoted 8 lines
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.

Agreed, the output needs to be fixed, backward compatibility is not an issue here.