[MPlayer-dev-eng] [PATCH] Add a function for parse and queue multiple cmds seperated by line-break

[Ulion]

2007/12/9, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> Hello,
> On Sun, Dec 09, 2007 at 08:36:01PM +0800, Ulion wrote:
> > 2007/12/9, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> [...]
> > > which also gets rid of the ugly "for(;;)".
> >
> > I ever read from somewhere, in the the compiled asm code, for(;;) just
> > loop, but while(1) still check.
>
> Maybe if you compile with -O0, but otherwise you wouldn't want to use a
> compiler that is _that_ stupid.
> But yes, e.g. Atmel still recommends such IMO really stupid things.
>
> > @@ -707,7 +708,28 @@
> >      mp_input_rm_key_fd(fd);
> >  }
> >
> > +int mp_input_parse_and_queue_cmds(const char *str) {
> > +    int cmd_num = 0;
> > +    mp_cmd_t *cmd;
> >
> > +    while (*str == '\n')
> > +        ++str;
> > +    while (*str) {
> > +        size_t len = strcspn(str, "\n");
> > +        char cmdbuf[len+1];
>
> Hmm... I think creating arbitrary-size arrays on the stack is a really
> bad idea, what does everyone else think?

I have no idea how the compiler handle such declaration. Indeed I
never known an array can have a dynamic size declaration before I
touch mplayer's code. Anyone can explain that to me?

>
> > +        av_strlcpy(cmdbuf, str, len+1);
> > +        cmd = mp_input_parse_cmd(cmdbuf);
> > +        if (cmd) {
> > +            mp_input_queue_cmd(cmd);
> > +            ++cmd_num;
> > +        }
>
> cmd is only used in the while loop, so you could move the declaration
> inside it. Doesn't matter much though.

Sure. Moved in since we can.

>
> > +        str += len;
> > +        while (*str == '\n')
> > +            ++str;
> > +    }
> > +    return cmd_num;
> > +}
>
> It might be worth handling \r as well as \n btw.
> The change would be simply replacing "\n" by "\n\r" in strcspn and
> "*str == '\n'" by "*str && strchr("\r\n", *str)".
> The *str check is because the specification seems unclear to me how
> strchr(str, 0); is supposed to behave.
> Well, I guess that makes "while (*str == '\n' || *str == '\r')" simpler
> than my strchr hack ;-)

\r supported. And also stripped heading spaces.

Updated patch is here.

-- 
Ulion
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: input_parse_and_queue_cmds3.diff
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20071209/81367bf6/attachment.txt>