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

Yuriy Kaminskiy yumkam at mail.ru
Thu Jun 10 23:21:54 CEST 2010


On 09.06.2010 22:25, Reimar Döffinger wrote:
> On Wed, Jun 09, 2010 at 08:02:50PM +0200, Reimar Döffinger wrote:
>> On Tue, Jun 08, 2010 at 10:55:08PM -0700, Howard Chu wrote:
>>>> Either way it seems obvious that is a non-productive way of attacking
>>>> our disagreement.
>>>> You have seen my proposal.
>>>> It IMO has at very least the advantage of being future-proof: If other
>>>> cases are added you don't have to take care to always read as much data
>>> >from s as you write to d, instead you just have to make sure you never write
>>>> more that 7 values.
>>>> This seems to me much easier to ensure.
>>>> In addition I think the check itself is more obvious than the memchr for '-'.
>>>> That it eliminates the need to assuming things about strtoul is also a bonus.
>>>> Do you disagree with those? Or do you have some other reason why you prefer
>>>> to do it the way you proposed?
>>> +	    else if (!strncmp (s, "&#", 2)) {
>>> +		uint32_t c;
>>> +		uint8_t tmp;
>>> +		q = s+2;
>>> +		c = strtoul(q, &s, 0);
BTW, why are you using base 0? It makes no sense.
I've seen Ȁ (unrecognized by your code, legal in HTML), but I've never
seen &#0x200;, and Ȁ HTML/XML/SGML parsers thinks is equivalent to
Ȁ/Ȁ (unlike your code, which think it is Ŋ/Ŋ).
I think this line should be
      c = *q == 'x' ? strtoul(q++, &s, 16) : strtoul(q, &s, 10);
>>> +		if (c <= 0x7fffffff) PUT_UTF8(c, tmp, *p++ = tmp;)

And I think malformed &# string should be copied as-is, not skipped.
Also I think it should handle &#0; in, uhm, less surprising way (either reject
or replace with ' ').
And I'm sure just decoding &#9;, &#10; and &#13; is not valid here.
>>> +		if (*s == ';') s++; }
>> So be pedantic, this assumes long is >= 32 bit and two's complement
> 
> After re-reading the spec I guess it probably won't.
> I still consider it a quite round-about way to check that
> s-q is less than what PUT_UTF8 will write, which in turn
> is already a non-obvious way to ensure that p will not
> go outside the array.




More information about the MPlayer-dev-eng mailing list