[MPlayer-dev-eng] [PATCH] fix SAMI parsing

[Reimar Döffinger]

On Sun, Jun 06, 2010 at 02:51:29PM -0700, Howard Chu wrote:
> Index: subreader.c
> ===================================================================
> --- subreader.c	(revision 31341)
> +++ subreader.c	(working copy)
> @@ -116,10 +116,12 @@
>      static char *s = NULL, *slacktime_s;
>      char text[LINE_LEN+1], *p=NULL, *q;
>      int state;
> +    extern int sub_utf8;

There's already one further down, you should just merge them.
I also think libvo/sub.h is the right way to get this..

>      current->lines = current->start = current->end = 0;
>      current->alignment = SUB_ALIGNMENT_BOTTOMCENTER;
>      state = 0;
> +    sub_utf8 = 1;

I suspect this will cause a memleak with -subcp
Somewhere around the
"Detected subtitle file format"
message might be better.
Of course a really nice long-term solution would later
make this more generic, and only make it the default
while the user can still override with -utf8 and -noutf8 -
but that's a bit much for this simple feature.

> @@ -161,17 +165,25 @@
>  
>  	case 3: /* get all text until '<' appears */
>  	    if (*s == '\0') break;
> -	    else if (!strncasecmp (s, "<br>", 4)) {
> +	    else if (!strncasecmp (s, "<br",  3) ||
> +		     !strncasecmp (s, "</p>", 4)) {
>  		*p = '\0'; p = text; trail_space (text);
>  		if (text[0] != '\0')
>  		    current->text[current->lines++] = strdup (text);
> -		s += 4;
> +		s = strchr (s, '>');
> +		if (!s) break;
> +		s++;

Not sure it's really better than the original solution, but I don't
have any particularly better suggestion.
Maybe add a comment though why the "<br", otherwise someone will think
the > was just forgotten.

> +	    else if (!strncasecmp (s, "&#", 2)) {

I've never seen uppercase (nor lowercase) versions of &#

> +		uint32_t c = strtol(s+2, &s, 0);
> +		uint8_t tmp;
> +		PUT_UTF8(c, tmp, *p++ = tmp;)
> +		if (*s == ';') s++; }

I'm sorry, but I think you'll have to somehow "prove"
this is not a security issue.
Checking and documenting that PUT_UTF8 will never write
more than we read might be possible.
Or just "blindly" checking we still have at least 8 bytes
free should do as well.