| Message ID | 20170402144909.51374-1-lq@chinaffmpeg.org |
|---|---|
| State | Superseded |
| Headers | show |
2017-04-02 22:49 GMT+08:00 Steven Liu <lq@chinaffmpeg.org>: > Use AVBprint to implementing av_strreplace > > Signed-off-by: Steven Liu <lq@chinaffmpeg.org> > --- > libavutil/avstring.c | 74 ++++++------------------------ > ---------------------- > 1 file changed, 8 insertions(+), 66 deletions(-) > > diff --git a/libavutil/avstring.c b/libavutil/avstring.c > index 52e6e6cd13..55f3bee94c 100644 > --- a/libavutil/avstring.c > +++ b/libavutil/avstring.c > @@ -233,78 +233,20 @@ int av_strncasecmp(const char *a, const char *b, > size_t n) > > char *av_strreplace(const char *str, const char *from, const char *to) > { > - /* Adjust each of the below values to suit your needs. */ > - /* Increment positions cache size initially by this number. */ > - size_t cache_sz_inc = 16; > - /* Thereafter, each time capacity needs to be increased, > - * multiply the increment by this factor. */ > - const size_t cache_sz_inc_factor = 3; > - /* But never increment capacity by more than this number. */ > - const size_t cache_sz_inc_max = 1048576; > - > - char *pret, *ret = NULL; > + char *ret = NULL; > const char *pstr2, *pstr = str; > - size_t i, count = 0; > - uintptr_t *pos_cache_tmp, *pos_cache = NULL; > - size_t cache_sz = 0; > - size_t cpylen, orglen, retlen, tolen, fromlen = strlen(from); > + size_t tolen = strlen(to), fromlen = strlen(from); > + AVBPrint pbuf; > > - /* Find all matches and cache their positions. */ > + av_bprint_init(&pbuf, 1, AV_BPRINT_SIZE_UNLIMITED); > while ((pstr2 = av_stristr(pstr, from))) { > - count++; > - /* Increase the cache size when necessary. */ > - if (cache_sz < count) { > - cache_sz += cache_sz_inc; > - pos_cache_tmp = av_realloc(pos_cache, sizeof(*pos_cache) * > cache_sz); > - if (!pos_cache_tmp) { > - goto end_strreplace; > - } else pos_cache = pos_cache_tmp; > - cache_sz_inc *= cache_sz_inc_factor; > - if (cache_sz_inc > cache_sz_inc_max) { > - cache_sz_inc = cache_sz_inc_max; > - } > - } > - > - pos_cache[count-1] = pstr2 - str; > + av_bprint_append_data(&pbuf, pstr, pstr2 - pstr); > pstr = pstr2 + fromlen; > + av_bprint_append_data(&pbuf, to, tolen); > } > - orglen = pstr - str + strlen(pstr); > - /* Allocate memory for the post-replacement string. */ > - if (count > 0) { > - tolen = strlen(to); > - retlen = orglen + (tolen - fromlen) * count; > - } else { > - retlen = orglen; > - } > - ret = av_malloc(retlen + 1); > - if (!ret) { > - goto end_strreplace; > - } > - > - if (!count) { > - /* If no matches, then just duplicate the string. */ > - av_strlcpy(ret, str, retlen + 1); > - } else { > - /* Otherwise, duplicate the string whilst performing > - * the replacements using the position cache. */ > - pret = ret; > - memcpy(pret, str, pos_cache[0]); > - pret += pos_cache[0]; > - for (i = 0; i < count; i++) { > - memcpy(pret, to, tolen); > - pret += tolen; > - pstr = str + pos_cache[i] + fromlen; > - cpylen = (i == count-1 ? orglen : pos_cache[i+1]) - > pos_cache[i] - fromlen; > - memcpy(pret, pstr, cpylen); > - pret += cpylen; > - } > - ret[retlen] = '\0'; > - } > + av_bprint_append_data(&pbuf, pstr, strlen(pstr)); > + av_bprint_finalize(&pbuf, &ret); > > -end_strreplace: > - /* Free the cache and return the post-replacement string, > - * which will be NULL in the event of an error. */ > - av_free(pos_cache); > return ret; > } > > -- > 2.11.0 (Apple Git-81) > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > I have implementation the av_strreplace use AVBprint, the API context now is: char *av_strreplace(const char *str, const char *from, const char *to) { char *ret = NULL; const char *pstr2, *pstr = str; size_t tolen = strlen(to), fromlen = strlen(from); AVBPrint pbuf; av_bprint_init(&pbuf, 1, AV_BPRINT_SIZE_UNLIMITED); while ((pstr2 = av_stristr(pstr, from))) { av_bprint_append_data(&pbuf, pstr, pstr2 - pstr); pstr = pstr2 + fromlen; av_bprint_append_data(&pbuf, to, tolen); } av_bprint_append_data(&pbuf, pstr, strlen(pstr)); av_bprint_finalize(&pbuf, &ret); return ret; }
Le tridi 13 germinal, an CCXXV, Steven Liu a écrit : > I have implementation the av_strreplace use AVBprint, the API context now > is: No need to Cc mo. On the other hand, sending the function itself instead of the diff makes it easier in this case, thanks. Nice. Just one issue: > char *av_strreplace(const char *str, const char *from, const char *to) > { > char *ret = NULL; > const char *pstr2, *pstr = str; > size_t tolen = strlen(to), fromlen = strlen(from); > AVBPrint pbuf; > > av_bprint_init(&pbuf, 1, AV_BPRINT_SIZE_UNLIMITED); > while ((pstr2 = av_stristr(pstr, from))) { > av_bprint_append_data(&pbuf, pstr, pstr2 - pstr); > pstr = pstr2 + fromlen; > av_bprint_append_data(&pbuf, to, tolen); > } > av_bprint_append_data(&pbuf, pstr, strlen(pstr)); > av_bprint_finalize(&pbuf, &ret); Just before that, a check with av_bprint_is_complete() is needed because some malloc may have failed: if it is not complete, av_bprint_finalize() to NULL to free the buffer and return NULL. > > return ret; > } Also, did you consider extending libavutil/tests/avstring.c to cover this new function? Regards,
2017-04-02 23:18 GMT+08:00 Nicolas George <george@nsup.org>: > Le tridi 13 germinal, an CCXXV, Steven Liu a écrit : > > I have implementation the av_strreplace use AVBprint, the API context now > > is: > > No need to Cc mo. On the other hand, sending the function itself instead > of the diff makes it easier in this case, thanks. > > Nice. Just one issue: > > > char *av_strreplace(const char *str, const char *from, const char *to) > > { > > char *ret = NULL; > > const char *pstr2, *pstr = str; > > size_t tolen = strlen(to), fromlen = strlen(from); > > AVBPrint pbuf; > > > > av_bprint_init(&pbuf, 1, AV_BPRINT_SIZE_UNLIMITED); > > while ((pstr2 = av_stristr(pstr, from))) { > > av_bprint_append_data(&pbuf, pstr, pstr2 - pstr); > > pstr = pstr2 + fromlen; > > av_bprint_append_data(&pbuf, to, tolen); > > } > > av_bprint_append_data(&pbuf, pstr, strlen(pstr)); > > > av_bprint_finalize(&pbuf, &ret); > > Just before that, a check with av_bprint_is_complete() is needed because > some malloc may have failed: if it is not complete, av_bprint_finalize() > to NULL to free the buffer and return NULL. > > > > > return ret; > > } > > Also, did you consider extending libavutil/tests/avstring.c to cover > this new function? > > Regards, > > -- > Nicolas George > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > What about this one: char *av_strreplace(const char *str, const char *from, const char *to) { char *ret = NULL; const char *pstr2, *pstr = str; size_t tolen = strlen(to), fromlen = strlen(from); AVBPrint pbuf; av_bprint_init(&pbuf, 1, AV_BPRINT_SIZE_UNLIMITED); while ((pstr2 = av_stristr(pstr, from))) { av_bprint_append_data(&pbuf, pstr, pstr2 - pstr); pstr = pstr2 + fromlen; av_bprint_append_data(&pbuf, to, tolen); } av_bprint_append_data(&pbuf, pstr, strlen(pstr)); if (!av_bprint_is_complete(&pbuf)) { av_bprint_finalize(&pbuf, NULL); } else { av_bprint_finalize(&pbuf, &ret); } return ret; } and i will update this patch, add TEST case. and update patch for patchwork.
diff --git a/libavutil/avstring.c b/libavutil/avstring.c index 52e6e6cd13..55f3bee94c 100644 --- a/libavutil/avstring.c +++ b/libavutil/avstring.c @@ -233,78 +233,20 @@ int av_strncasecmp(const char *a, const char *b, size_t n) char *av_strreplace(const char *str, const char *from, const char *to) { - /* Adjust each of the below values to suit your needs. */ - /* Increment positions cache size initially by this number. */ - size_t cache_sz_inc = 16; - /* Thereafter, each time capacity needs to be increased, - * multiply the increment by this factor. */ - const size_t cache_sz_inc_factor = 3; - /* But never increment capacity by more than this number. */ - const size_t cache_sz_inc_max = 1048576; - - char *pret, *ret = NULL; + char *ret = NULL; const char *pstr2, *pstr = str; - size_t i, count = 0; - uintptr_t *pos_cache_tmp, *pos_cache = NULL; - size_t cache_sz = 0; - size_t cpylen, orglen, retlen, tolen, fromlen = strlen(from); + size_t tolen = strlen(to), fromlen = strlen(from); + AVBPrint pbuf; - /* Find all matches and cache their positions. */ + av_bprint_init(&pbuf, 1, AV_BPRINT_SIZE_UNLIMITED); while ((pstr2 = av_stristr(pstr, from))) { - count++; - /* Increase the cache size when necessary. */ - if (cache_sz < count) { - cache_sz += cache_sz_inc; - pos_cache_tmp = av_realloc(pos_cache, sizeof(*pos_cache) * cache_sz); - if (!pos_cache_tmp) { - goto end_strreplace; - } else pos_cache = pos_cache_tmp; - cache_sz_inc *= cache_sz_inc_factor; - if (cache_sz_inc > cache_sz_inc_max) { - cache_sz_inc = cache_sz_inc_max; - } - } - - pos_cache[count-1] = pstr2 - str; + av_bprint_append_data(&pbuf, pstr, pstr2 - pstr); pstr = pstr2 + fromlen; + av_bprint_append_data(&pbuf, to, tolen); } - orglen = pstr - str + strlen(pstr); - /* Allocate memory for the post-replacement string. */ - if (count > 0) { - tolen = strlen(to); - retlen = orglen + (tolen - fromlen) * count; - } else { - retlen = orglen; - } - ret = av_malloc(retlen + 1); - if (!ret) { - goto end_strreplace; - } - - if (!count) { - /* If no matches, then just duplicate the string. */ - av_strlcpy(ret, str, retlen + 1); - } else { - /* Otherwise, duplicate the string whilst performing - * the replacements using the position cache. */ - pret = ret; - memcpy(pret, str, pos_cache[0]); - pret += pos_cache[0]; - for (i = 0; i < count; i++) { - memcpy(pret, to, tolen); - pret += tolen; - pstr = str + pos_cache[i] + fromlen; - cpylen = (i == count-1 ? orglen : pos_cache[i+1]) - pos_cache[i] - fromlen; - memcpy(pret, pstr, cpylen); - pret += cpylen; - } - ret[retlen] = '\0'; - } + av_bprint_append_data(&pbuf, pstr, strlen(pstr)); + av_bprint_finalize(&pbuf, &ret); -end_strreplace: - /* Free the cache and return the post-replacement string, - * which will be NULL in the event of an error. */ - av_free(pos_cache); return ret; }
Use AVBprint to implementing av_strreplace Signed-off-by: Steven Liu <lq@chinaffmpeg.org> --- libavutil/avstring.c | 74 ++++++---------------------------------------------- 1 file changed, 8 insertions(+), 66 deletions(-)