[FFmpeg-devel] [PATCH v1] avcodec/bsf: simplify the code

[Paul B Mahol]

On 4/17/20, Limin Wang <lance.lmwang at gmail.com> wrote:
> On Fri, Apr 17, 2020 at 04:41:44PM +0200, Andreas Rheinhardt wrote:
>> lance.lmwang at gmail.com:
>> > From: Limin Wang <lance.lmwang at gmail.com>
>> >
>> > Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
>> > ---
>> >  libavcodec/bsf.c | 10 ++--------
>> >  1 file changed, 2 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/libavcodec/bsf.c b/libavcodec/bsf.c
>> > index 7b96183e64..c4c939c205 100644
>> > --- a/libavcodec/bsf.c
>> > +++ b/libavcodec/bsf.c
>> > @@ -533,7 +533,7 @@ end:
>> >  int av_bsf_list_parse_str(const char *str, AVBSFContext **bsf_lst)
>> >  {
>> >      AVBSFList *lst;
>> > -    char *bsf_str, *buf, *dup, *saveptr;
>> > +    char *bsf_str, *buf, *dup;
>> >      int ret;
>> >
>> >      if (!str)
>> > @@ -548,16 +548,10 @@ int av_bsf_list_parse_str(const char *str,
>> > AVBSFContext **bsf_lst)
>> >          goto end;
>> >      }
>> >
>> > -    while (1) {
>> > -        bsf_str = av_strtok(buf, ",", &saveptr);
>> > -        if (!bsf_str)
>> > -            break;
>> > -
>> > +    while (bsf_str = av_strtok(buf, ",", &buf)) {
>> >          ret = bsf_parse_single(bsf_str, lst);
>> >          if (ret < 0)
>> >              goto end;
>> > -
>> > -        buf = NULL;
>> >      }
>> >
>> >      ret = av_bsf_list_finalize(&lst, bsf_lst);
>> >
>> This is against the documentation of av_strtok() which states:
>>  * On the first call to av_strtok(), s should point to the string to
>>  * parse, and the value of saveptr is ignored. In subsequent calls, s
>>  * should be NULL, and saveptr should be unchanged since the previous
>>  * call.
>>
>> It works now, but it is not guaranteed to work.
>
> I don't know why the subsequent calls, s should be NULL. I think it's
> willing,
> not must. If we're clear, why not to make the buf point to the next token,
> it
> looks more simple and easy to read.
>
> Also, a lot of such case have used in existing code.
>
> [lmwang at vpn ffmpeg]$ grep av_strtok libavformat/*.c |grep while
> libavformat/dashdec.c:    while (mpdName = av_strtok(tmp, "/", &tmp))  {
> libavformat/ftp.c:    while(fact = av_strtok(mlsd, ";", &mlsd)) {
> libavformat/http.c:    while ((param = av_strtok(next_param, ";",
> &next_param))) {
> libavformat/http.c:    while ((cookie = av_strtok(next, "\n", &next)) &&
> !ret) {
>

Thanks for listing invalid code lines.

>
>>
>> - Andreas
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>
> --
> Thanks,
> Limin Wang
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".