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

[Limin Wang]

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) {


> 
> - 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