Re: [FFmpeg-devel] [PATCH v3 1/2] avformat/url: rework for trim_double_dot_url
Generally, this seems to be too complicated with too much of copying. On Mon, Jul 27, 2020 at 3:08 PM Steven Liu wrote: > use two buffer for storage the path and node of URI > remove the last node of the path if the next node is .. > change the static strings to dynamic alloc space by size argument. > > Signed-off-by: Steven Liu > --- > libavformat/url.c | 83 --- > 1 file changed, 64 insertions(+), 19 deletions(-) > > diff --git a/libavformat/url.c b/libavformat/url.c > index 20463a6674..4fb703de78 100644 > --- a/libavformat/url.c > +++ b/libavformat/url.c > @@ -82,41 +82,78 @@ static void trim_double_dot_url(char *buf, const char > *rel, int size) > { > const char *p = rel; > const char *root = rel; > -char tmp_path[MAX_URL_SIZE] = {0, }; > +char *tmp_path = NULL; > +char *second_buf = NULL; > +char *first_buf = NULL; > char *sep; > char *node; > > +tmp_path = av_mallocz(size); > +if (!tmp_path) > +return; > + > +second_buf = av_mallocz(size); > +if (!second_buf) > +goto fail; > + > +first_buf = av_mallocz(size); > +if (!first_buf) > +goto fail; > Why 3 buffers? 1 would be enough. + > /* Get the path root of the url which start by "://" */ > if (p && (sep = strstr(p, "://"))) { > sep += 3; > root = strchr(sep, '/'); > if (!root) > -return; > +goto fail; > } > +av_strlcat(tmp_path, p, root - p + 1); > > /* set new current position if the root node is changed */ > p = root; > -while (p && (node = strstr(p, ".."))) { > -av_strlcat(tmp_path, p, node - p + strlen(tmp_path)); > -p = node + 3; > -sep = strrchr(tmp_path, '/'); > +if (*p == '/') > +p++; > +while (p && (node = strchr(p, '/'))) { > +av_strlcpy(second_buf, p, node - p + 1); > +p = node + 1; > +if (!strcmp(second_buf, "..")) { > If you used if(node - p == 2 && !strncmp(node, "..", 2)) you would not have to copy to the second buffer. +if (strlen(first_buf) <= 0) > +continue; > +sep = strrchr(first_buf, '/'); > +if (sep) > +sep[0] = '\0'; > +else > +memset(first_buf, 0, size); > + > +memset(second_buf, 0, size); > +} else { > +av_strlcat(first_buf, "/", size); > +av_strlcat(first_buf, second_buf, size); > +} > +} > +if (!strcmp(p, "..")) { > +sep = strrchr(first_buf, '/'); > if (sep) > sep[0] = '\0'; > else > -tmp_path[0] = '\0'; > +memset(first_buf, 0, size); > +} else { > +av_strlcat(first_buf, "/", size); > +av_strlcat(first_buf, p, size); > } > > -if (!av_stristart(p, "/", NULL) && root != rel) > -av_strlcat(tmp_path, "/", size); > - > -av_strlcat(tmp_path, p, size); > -/* start set buf after temp path process. */ > -av_strlcpy(buf, rel, root - rel + 1); > - > -if (!av_stristart(tmp_path, "/", NULL) && root != rel) > -av_strlcat(buf, "/", size); > +if (p > root) { > +av_strlcat(tmp_path, first_buf, size); > +} else { > +av_strlcat(tmp_path, p, size); > +} > > av_strlcat(buf, tmp_path, size); > + > +fail: > +av_free(second_buf); > +av_free(first_buf); > +av_free(tmp_path); > } > > void ff_make_absolute_url(char *buf, int size, const char *base, > @@ -124,9 +161,11 @@ void ff_make_absolute_url(char *buf, int size, const > char *base, > { > char *sep, *path_query; > char *root, *p; > -char tmp_path[MAX_URL_SIZE]; > +char *tmp_path = av_mallocz(size); > + > +if (!tmp_path) > +return; > > -memset(tmp_path, 0, sizeof(tmp_path)); > /* Absolute path, relative to the current server */ > if (base && strstr(base, "://") && rel[0] == '/') { > if (base != buf) > @@ -148,12 +187,14 @@ void ff_make_absolute_url(char *buf, int size, const > char *base, > trim_double_dot_url(tmp_path, buf, size); > memset(buf, 0, size); > av_strlcpy(buf, tmp_path, size); > +av_free(tmp_path); > return; > } > /* If rel actually is an absolute url, just copy it */ > if (!base || strstr(rel, "://") || rel[0] == '/') { > memset(buf, 0, size); > trim_double_dot_url(buf, rel, size); > +av_free(tmp_path); > return; > } > if (base != buf) > @@ -170,6 +211,7 @@ void ff_make_absolute_url(char *buf, int size, const > char *base, > trim_double_dot_url(tmp_path, buf, size); > memset(buf, 0, size); > av_strlcpy(buf, tmp_path, size); > +av_free(tmp_path); > return; > } > > @@ -180,8 +222,10 @@ void ff_make_absolute_url(char *buf, int size, const > char *base, > if (sep) { >
Re: [FFmpeg-devel] [PATCH v3 1/2] avformat/url: rework for trim_double_dot_url
Steven Liu (12020-07-27): > use two buffer for storage the path and node of URI > remove the last node of the path if the next node is .. > change the static strings to dynamic alloc space by size argument. ?!?!??!!!? Why do you think you need extra buffers? This patch turns a rather straightforward task into a nightmare of string copies. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 1/2] avformat/url: rework for trim_double_dot_url
use two buffer for storage the path and node of URI remove the last node of the path if the next node is .. change the static strings to dynamic alloc space by size argument. Signed-off-by: Steven Liu --- libavformat/url.c | 83 --- 1 file changed, 64 insertions(+), 19 deletions(-) diff --git a/libavformat/url.c b/libavformat/url.c index 20463a6674..4fb703de78 100644 --- a/libavformat/url.c +++ b/libavformat/url.c @@ -82,41 +82,78 @@ static void trim_double_dot_url(char *buf, const char *rel, int size) { const char *p = rel; const char *root = rel; -char tmp_path[MAX_URL_SIZE] = {0, }; +char *tmp_path = NULL; +char *second_buf = NULL; +char *first_buf = NULL; char *sep; char *node; +tmp_path = av_mallocz(size); +if (!tmp_path) +return; + +second_buf = av_mallocz(size); +if (!second_buf) +goto fail; + +first_buf = av_mallocz(size); +if (!first_buf) +goto fail; + /* Get the path root of the url which start by "://" */ if (p && (sep = strstr(p, "://"))) { sep += 3; root = strchr(sep, '/'); if (!root) -return; +goto fail; } +av_strlcat(tmp_path, p, root - p + 1); /* set new current position if the root node is changed */ p = root; -while (p && (node = strstr(p, ".."))) { -av_strlcat(tmp_path, p, node - p + strlen(tmp_path)); -p = node + 3; -sep = strrchr(tmp_path, '/'); +if (*p == '/') +p++; +while (p && (node = strchr(p, '/'))) { +av_strlcpy(second_buf, p, node - p + 1); +p = node + 1; +if (!strcmp(second_buf, "..")) { +if (strlen(first_buf) <= 0) +continue; +sep = strrchr(first_buf, '/'); +if (sep) +sep[0] = '\0'; +else +memset(first_buf, 0, size); + +memset(second_buf, 0, size); +} else { +av_strlcat(first_buf, "/", size); +av_strlcat(first_buf, second_buf, size); +} +} +if (!strcmp(p, "..")) { +sep = strrchr(first_buf, '/'); if (sep) sep[0] = '\0'; else -tmp_path[0] = '\0'; +memset(first_buf, 0, size); +} else { +av_strlcat(first_buf, "/", size); +av_strlcat(first_buf, p, size); } -if (!av_stristart(p, "/", NULL) && root != rel) -av_strlcat(tmp_path, "/", size); - -av_strlcat(tmp_path, p, size); -/* start set buf after temp path process. */ -av_strlcpy(buf, rel, root - rel + 1); - -if (!av_stristart(tmp_path, "/", NULL) && root != rel) -av_strlcat(buf, "/", size); +if (p > root) { +av_strlcat(tmp_path, first_buf, size); +} else { +av_strlcat(tmp_path, p, size); +} av_strlcat(buf, tmp_path, size); + +fail: +av_free(second_buf); +av_free(first_buf); +av_free(tmp_path); } void ff_make_absolute_url(char *buf, int size, const char *base, @@ -124,9 +161,11 @@ void ff_make_absolute_url(char *buf, int size, const char *base, { char *sep, *path_query; char *root, *p; -char tmp_path[MAX_URL_SIZE]; +char *tmp_path = av_mallocz(size); + +if (!tmp_path) +return; -memset(tmp_path, 0, sizeof(tmp_path)); /* Absolute path, relative to the current server */ if (base && strstr(base, "://") && rel[0] == '/') { if (base != buf) @@ -148,12 +187,14 @@ void ff_make_absolute_url(char *buf, int size, const char *base, trim_double_dot_url(tmp_path, buf, size); memset(buf, 0, size); av_strlcpy(buf, tmp_path, size); +av_free(tmp_path); return; } /* If rel actually is an absolute url, just copy it */ if (!base || strstr(rel, "://") || rel[0] == '/') { memset(buf, 0, size); trim_double_dot_url(buf, rel, size); +av_free(tmp_path); return; } if (base != buf) @@ -170,6 +211,7 @@ void ff_make_absolute_url(char *buf, int size, const char *base, trim_double_dot_url(tmp_path, buf, size); memset(buf, 0, size); av_strlcpy(buf, tmp_path, size); +av_free(tmp_path); return; } @@ -180,8 +222,10 @@ void ff_make_absolute_url(char *buf, int size, const char *base, if (sep) { sep += 3; root = strchr(sep, '/'); -if (!root) +if (!root) { +av_free(tmp_path); return; +} } } @@ -218,6 +262,7 @@ void ff_make_absolute_url(char *buf, int size, const char *base, trim_double_dot_url(tmp_path, buf, size); memset(buf, 0, size); av_strlcpy(buf, tmp_path, size); +av_free(tmp_path); } AVIODirEntry *ff_alloc_dir_entry(void) -- 2.25.0 ___ ffmpeg-devel mailing list