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

[Reimar Döffinger]

On Wed, Jun 09, 2010 at 05:31:50PM -0700, Howard Chu wrote:
> 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);
> >>>+		if (c<= 0x7fffffff) PUT_UTF8(c, tmp, *p++ = tmp;)
> >>>+		if (*s == ';') s++; }
> >>
> >>So be pedantic, this assumes long is>= 32 bit and two's complement
> >
> >And certainly doesn't answer my question as to the "why".
> 
> >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.
> 
> Your approach with a fixed 8-byte check was clearly wrong. It would
> falsely detect out-of-buffer space for a wide range of valid/safe
> inputs, e.g. "&#32;".

Sure, but only if the line already contains around 1000 characters.
We can just increase LINE_LEN if that's an issue...