2015-04-07 13:12 UTC+02:00, Sebastian Ramacher <[email protected]>: > On 2015-04-06 22:33:28, Celelibi wrote: >> Package: yafc >> Version: 1.3.5-2 >> Severity: important >> Tags: upstream patch >> >> Dear maintainer, >> >> There is a bug in the path shortener that makes yafc crash on some >> specific input. >> >> When it tries to replace a short directory name with "..." it tries to >> push the remaining of the string farther away, without reallocating a >> larger buffer. >> >> For instance: >> /w/some/very/long/pathname/whichis/really/too/long/andshouldbeshortened.html >> is likely to cause the bug. >> >> The actual bug is in src/libmhe/shortpath.c when using strpush. >> >> I propose here a patch that simplifies the path shortening code so that >> it has no loop, no complex condition, no temporary memory allocation, no >> complex string handling. >> >> In short: half the size, twice the fun. :) >> >> The patch hasn't been extensively tested, but I guess it should be >> bug-free and behave like the original code. > > The behavior of the new code differs from the old code. Previously, > "averylongdirectoryname/filename.extension" was shortened to > "...me/filename.extension". The new code returns > "averylongdirectoryname/...t".
True, this patch was also buggy in some cases. :) > Also, "/usr/bin/averylongfilenamethatistoolongindeed.tar.gz = > ...stoolongindeed.tar.gz" was shortened to "...stoolongindeed.tar.gz" and > now > it's "/...stoolongindeed.tar.gz". That's true too. I'll fix that, but notice that this is inconsistent with the comment above the current code that say: /usr/bin/averylongfilenamethatistoolongindeed.tar.gz => /...oolongindeed.tar.gz Since I could decide exactly what behavior was expected, it's not 1, not 2, but 3 patches to choose from that I send there. :) - The first one (0001-shortpath-exact.patch) just try to mimic the behavior of the original code. The only difference in behavior that I could observe is that it doesn't generate some silly "....../file.txt" that the current code does sometime when a previous "..." collide with the new one. - The second one (0001-shortpath-short-dir.patch) improves a bit the first one by handling better short directory names. For instance, "longdirectory/a/file.txt" doesn't get shortened to "...ngdir/.../thefile.txt" but to "...longdir/a/thefile.txt". Same goes for "a/some/dirs/goes/here/file.txt" which never gets shortened to ".../.../here/file.txt", but to "a/.../goes/here/file.txt". - The third one (0001-shortpath-simple.patch) completely remove that double "..." that can appear (like in the previous examples). This help a lot with code clarity and simplicity. I tried to test my patches as much as possible, but I'm still just a human. :) Best regards, Celelibi
From 53ac4ebc449af6b2d02dcc3c421273153ba70e9f Mon Sep 17 00:00:00 2001 From: Celelibi <[email protected]> Date: Wed, 8 Apr 2015 19:58:24 +0200 Subject: [PATCH] shortpath: simpler code Code shorter, simple, without loops. Signed-off-by: Celelibi <[email protected]> --- src/libmhe/shortpath.c | 77 +++++++++++++++++++++++--------------------------- 1 file changed, 35 insertions(+), 42 deletions(-) diff --git a/src/libmhe/shortpath.c b/src/libmhe/shortpath.c index 12a46d7..35f67d1 100644 --- a/src/libmhe/shortpath.c +++ b/src/libmhe/shortpath.c @@ -22,54 +22,47 @@ * /usr/bin/averylongfilenamethatistoolongindeed.tar.gz => /...oolongindeed.tar.gz */ -static char* chop(const char *str, size_t maxlen) +static char *_shortpath(char *path, size_t maxlen) { - const size_t len = strlen(str); + size_t len = strlen(path); + char *result = NULL; + const char *copy_from = NULL; + const char *first_slash = NULL; + const char *last_slash = NULL; + size_t start_len = 0; + size_t last_len = 0; + if (len <= maxlen) - return xstrdup(str); + return xstrdup(path); - char* result = malloc(maxlen + 1); - strncpy(result, "...", 3); - strncpy(result + 3, str+(len-maxlen+3), maxlen - 3); - result[maxlen] = '\0'; - return result; -} + result = xmalloc(maxlen + 1); + result[0] = '\0'; -static char* dirtodots(const char *path, size_t maxlen) -{ - const char* first_slash = strchr(path, '/'); - if (!first_slash) - return chop(path, maxlen); - - const char* end_slash = NULL; - if (strncmp(first_slash+1, "...", 3) == 0) - end_slash = strchr(first_slash+5, '/'); - else - end_slash = strchr(first_slash+1, '/'); - - if (!end_slash) - return chop(path, maxlen); - - size_t first = first_slash - path, - end = end_slash - path; - char* result = xstrdup(path); - if (end - first < 4) /* /fu/ */ - strpush(result + first +1, 4 - (end - first)); - else /* /foobar/ */ - strcpy(result + first + 4, end_slash); - strncpy(result + first + 1, "...", 3); - return result; -} + first_slash = strchr(path, '/'); + if (first_slash) + start_len = first_slash - path + 1; -static char *_shortpath(char *path, size_t maxlen) -{ - const size_t len = strlen(path); - if (len <= maxlen) - return xstrdup(path); + last_slash = strrchr(path + start_len + 1, '/'); + if (last_slash) + last_len = strlen(last_slash); + + if (last_slash && start_len + 3 + last_len <= maxlen) + strncpy(result, path, start_len); + + if (last_slash && start_len + 3 + last_len > maxlen && last_len + 7 <= maxlen) { + size_t lead_len = maxlen - (last_len + 3) - 3; + strcpy(result, "..."); + strncat(result, first_slash - lead_len + 1, lead_len); + start_len = lead_len + 3; + } + + copy_from = strchr(path + len - (maxlen - start_len - 3), '/'); + if (!copy_from) + copy_from = path + len - (maxlen - 3); + + strcat(result, "..."); + strcat(result, copy_from); - char* tmp = dirtodots(path, maxlen); - char* result = _shortpath(tmp, maxlen); - free(tmp); return result; } -- 2.1.4
From 5bd8ef62b85ef4440193cd6b05da23ca4b2e125a Mon Sep 17 00:00:00 2001 From: Celelibi <[email protected]> Date: Wed, 8 Apr 2015 19:58:24 +0200 Subject: [PATCH] shortpath: simpler code Code shorter, simple, without loops, handles short directories better. Signed-off-by: Celelibi <[email protected]> --- src/libmhe/shortpath.c | 80 ++++++++++++++++++++++++-------------------------- 1 file changed, 38 insertions(+), 42 deletions(-) diff --git a/src/libmhe/shortpath.c b/src/libmhe/shortpath.c index 12a46d7..c54afe9 100644 --- a/src/libmhe/shortpath.c +++ b/src/libmhe/shortpath.c @@ -22,54 +22,50 @@ * /usr/bin/averylongfilenamethatistoolongindeed.tar.gz => /...oolongindeed.tar.gz */ -static char* chop(const char *str, size_t maxlen) +static char *_shortpath(char *path, size_t maxlen) { - const size_t len = strlen(str); + size_t len = strlen(path); + char *result = NULL; + const char *copy_from = NULL; + const char *first_slash = NULL; + const char *last_slash = NULL; + size_t start_len = 0; + if (len <= maxlen) - return xstrdup(str); + return xstrdup(path); - char* result = malloc(maxlen + 1); - strncpy(result, "...", 3); - strncpy(result + 3, str+(len-maxlen+3), maxlen - 3); - result[maxlen] = '\0'; - return result; -} + result = xmalloc(maxlen + 1); + result[0] = '\0'; -static char* dirtodots(const char *path, size_t maxlen) -{ - const char* first_slash = strchr(path, '/'); - if (!first_slash) - return chop(path, maxlen); - - const char* end_slash = NULL; - if (strncmp(first_slash+1, "...", 3) == 0) - end_slash = strchr(first_slash+5, '/'); - else - end_slash = strchr(first_slash+1, '/'); - - if (!end_slash) - return chop(path, maxlen); - - size_t first = first_slash - path, - end = end_slash - path; - char* result = xstrdup(path); - if (end - first < 4) /* /fu/ */ - strpush(result + first +1, 4 - (end - first)); - else /* /foobar/ */ - strcpy(result + first + 4, end_slash); - strncpy(result + first + 1, "...", 3); - return result; -} + first_slash = strchr(path, '/'); + if (first_slash) + start_len = first_slash - path + 1; -static char *_shortpath(char *path, size_t maxlen) -{ - const size_t len = strlen(path); - if (len <= maxlen) - return xstrdup(path); + // Do not try to shorten short directory name + if (start_len + 5 < len) + last_slash = strrchr(path + start_len + 4, '/'); + + if (last_slash) { + size_t last_len = 0; + last_len = strlen(last_slash); + + if (start_len + 3 + last_len <= maxlen) { + strncpy(result, path, start_len); + } else if (last_len + min(start_len, 4) + 3 <= maxlen) { + size_t lead_len = maxlen - (last_len + 3) - 3; + strcpy(result, "..."); + strncat(result, first_slash - lead_len + 1, lead_len); + start_len = lead_len + 3; + } + } + + copy_from = strchr(path + len - (maxlen - start_len - 3), '/'); + if (!copy_from) + copy_from = path + len - (maxlen - 3); + + strcat(result, "..."); + strcat(result, copy_from); - char* tmp = dirtodots(path, maxlen); - char* result = _shortpath(tmp, maxlen); - free(tmp); return result; } -- 2.1.4
From 2ed5951e3b8b0ba465fe2d343e19bd94548e6233 Mon Sep 17 00:00:00 2001 From: Celelibi <[email protected]> Date: Wed, 8 Apr 2015 19:58:24 +0200 Subject: [PATCH] shortpath: simpler code Code shorter, simple, without loops, handles short directories better. Doesn't insert multiples "...". Signed-off-by: Celelibi <[email protected]> --- src/libmhe/shortpath.c | 61 +++++++++++++++++--------------------------------- 1 file changed, 21 insertions(+), 40 deletions(-) diff --git a/src/libmhe/shortpath.c b/src/libmhe/shortpath.c index 12a46d7..1bce60a 100644 --- a/src/libmhe/shortpath.c +++ b/src/libmhe/shortpath.c @@ -22,54 +22,35 @@ * /usr/bin/averylongfilenamethatistoolongindeed.tar.gz => /...oolongindeed.tar.gz */ -static char* chop(const char *str, size_t maxlen) +static char *_shortpath(char *path, size_t maxlen) { - const size_t len = strlen(str); + size_t len = strlen(path); + char *result = NULL; + const char *copy_from = NULL; + const char *first_slash = NULL; + size_t start_len = 0; + if (len <= maxlen) - return xstrdup(str); + return xstrdup(path); - char* result = malloc(maxlen + 1); - strncpy(result, "...", 3); - strncpy(result + 3, str+(len-maxlen+3), maxlen - 3); - result[maxlen] = '\0'; - return result; -} + result = xmalloc(maxlen + 1); + result[0] = '\0'; -static char* dirtodots(const char *path, size_t maxlen) -{ - const char* first_slash = strchr(path, '/'); - if (!first_slash) - return chop(path, maxlen); + first_slash = strchr(path, '/'); + if (first_slash) + start_len = first_slash - path + 1; - const char* end_slash = NULL; - if (strncmp(first_slash+1, "...", 3) == 0) - end_slash = strchr(first_slash+5, '/'); + if (maxlen - start_len - 3 > 0) + copy_from = strchr(path + len - (maxlen - start_len - 3), '/'); + + if (copy_from) + strncpy(result, path, start_len); else - end_slash = strchr(first_slash+1, '/'); - - if (!end_slash) - return chop(path, maxlen); - - size_t first = first_slash - path, - end = end_slash - path; - char* result = xstrdup(path); - if (end - first < 4) /* /fu/ */ - strpush(result + first +1, 4 - (end - first)); - else /* /foobar/ */ - strcpy(result + first + 4, end_slash); - strncpy(result + first + 1, "...", 3); - return result; -} + copy_from = path + len - (maxlen - 3); -static char *_shortpath(char *path, size_t maxlen) -{ - const size_t len = strlen(path); - if (len <= maxlen) - return xstrdup(path); + strcat(result, "..."); + strcat(result, copy_from); - char* tmp = dirtodots(path, maxlen); - char* result = _shortpath(tmp, maxlen); - free(tmp); return result; } -- 2.1.4
