| Message ID | 20161101080147.13163-2-quwenruo@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Nov 01, 2016 at 04:01:43PM +0800, Qu Wenruo wrote: > Introduce new function, escape_string_inplace(), to escape specified > characters in place. Sorry, the pointer to seq_path was misleading. The actual escape function is mangle_path and it copies one string to another. As we just print the path, we can simply switch and call putchar. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
At 11/01/2016 06:08 PM, David Sterba wrote: > On Tue, Nov 01, 2016 at 04:01:43PM +0800, Qu Wenruo wrote: >> Introduce new function, escape_string_inplace(), to escape specified >> characters in place. > > Sorry, the pointer to seq_path was misleading. The actual escape > function is mangle_path and it copies one string to another. As we just > print the path, we can simply switch and call putchar. > Putchar() method is indeed much easier to implement. But it makes us hard to do further formatting, like aligning the path to given width. (At least we are still using 32 chars alignment for path) So I still prefer the current full function string escaping and still use %-32s for formatting. And the idea of implementing escape_string_inplace() as a pure string manipulation function can make it more agile for later use. For example, we can reuse it for print-tree. Although the current esacpe_string_inplace() can be further fine-tuned to avoid calling memmove() in a loop, which I can update it in next version. Thanks, Qu -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Nov 02, 2016 at 09:19:10AM +0800, Qu Wenruo wrote: > > > At 11/01/2016 06:08 PM, David Sterba wrote: > > On Tue, Nov 01, 2016 at 04:01:43PM +0800, Qu Wenruo wrote: > >> Introduce new function, escape_string_inplace(), to escape specified > >> characters in place. > > > > Sorry, the pointer to seq_path was misleading. The actual escape > > function is mangle_path and it copies one string to another. As we just > > print the path, we can simply switch and call putchar. > > > > Putchar() method is indeed much easier to implement. > > But it makes us hard to do further formatting, like aligning the path to > given width. (At least we are still using 32 chars alignment for path) > > So I still prefer the current full function string escaping and still > use %-32s for formatting. > > > And the idea of implementing escape_string_inplace() as a pure string > manipulation function can make it more agile for later use. > For example, we can reuse it for print-tree. Reusing is fine, but I really don't like that the function modifies the argument. What if the function is called twice on the same string? Also, in the print-tree, this would mean the extent buffer would be modified, potentially overwriting other items. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
At 11/02/2016 06:55 PM, David Sterba wrote: > On Wed, Nov 02, 2016 at 09:19:10AM +0800, Qu Wenruo wrote: >> >> >> At 11/01/2016 06:08 PM, David Sterba wrote: >>> On Tue, Nov 01, 2016 at 04:01:43PM +0800, Qu Wenruo wrote: >>>> Introduce new function, escape_string_inplace(), to escape specified >>>> characters in place. >>> >>> Sorry, the pointer to seq_path was misleading. The actual escape >>> function is mangle_path and it copies one string to another. As we just >>> print the path, we can simply switch and call putchar. >>> >> >> Putchar() method is indeed much easier to implement. >> >> But it makes us hard to do further formatting, like aligning the path to >> given width. (At least we are still using 32 chars alignment for path) >> >> So I still prefer the current full function string escaping and still >> use %-32s for formatting. >> >> >> And the idea of implementing escape_string_inplace() as a pure string >> manipulation function can make it more agile for later use. >> For example, we can reuse it for print-tree. > > Reusing is fine, but I really don't like that the function modifies the > argument. What if the function is called twice on the same string? Also, > in the print-tree, this would mean the extent buffer would be modified, > potentially overwriting other items. > > Then just encapsulate the in-place version into memory allocation version. In-place version can be encapsulated very easily, while it's not possible vice verse. So there will be two functions, escape_string_inplace() and escape_string() for caller to choose. Would this be OK? Thanks, Qu -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
At 11/02/2016 06:55 PM, David Sterba wrote: > On Wed, Nov 02, 2016 at 09:19:10AM +0800, Qu Wenruo wrote: >> >> >> At 11/01/2016 06:08 PM, David Sterba wrote: >>> On Tue, Nov 01, 2016 at 04:01:43PM +0800, Qu Wenruo wrote: >>>> Introduce new function, escape_string_inplace(), to escape specified >>>> characters in place. >>> >>> Sorry, the pointer to seq_path was misleading. The actual escape >>> function is mangle_path and it copies one string to another. As we just >>> print the path, we can simply switch and call putchar. >>> >> >> Putchar() method is indeed much easier to implement. >> >> But it makes us hard to do further formatting, like aligning the path to >> given width. (At least we are still using 32 chars alignment for path) >> >> So I still prefer the current full function string escaping and still >> use %-32s for formatting. >> >> >> And the idea of implementing escape_string_inplace() as a pure string >> manipulation function can make it more agile for later use. >> For example, we can reuse it for print-tree. > > Reusing is fine, but I really don't like that the function modifies the > argument. What if the function is called twice on the same string? Forgot to ask, what's the problem calling the function twice on the same string? escape_string_inplace(dest, " "); escape_string_inplace(dest, "\n"); is just the same as escape_string_inplace(dest, " \n"); So I see no problem here. Thanks, Qu -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/utils.c b/utils.c index 3f54245..69faae1 100644 --- a/utils.c +++ b/utils.c @@ -4251,3 +4251,20 @@ unsigned int rand_range(unsigned int upper) */ return (unsigned int)(jrand48(rand_seed) % upper); } + +void string_escape_inplace(char *restrict string, char *restrict escape_chars) +{ + int i, j; + + for (i = 0; i < strlen(escape_chars); i++) { + j = 0; + while (j < strlen(string)) { + if (string[j] == escape_chars[i]) { + memmove(string + j, string + j + 1, + strlen(string) -j); + continue; + } + j++; + } + } +} diff --git a/utils.h b/utils.h index 1a2dbcd..b36d411 100644 --- a/utils.h +++ b/utils.h @@ -457,4 +457,6 @@ unsigned int rand_range(unsigned int upper); /* Also allow setting the seed manually */ void init_rand_seed(u64 seed); +void string_escape_inplace(char *restrict string, char *restrict escape_chars); + #endif
Introduce new function, escape_string_inplace(), to escape specified characters in place. Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> --- utils.c | 17 +++++++++++++++++ utils.h | 2 ++ 2 files changed, 19 insertions(+)