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

[Reimar Döffinger]

On Mon, Jun 07, 2010 at 11:47:46PM -0700, Howard Chu wrote:
> Reimar Döffinger wrote:
> >>You gave a completely valid criticism - negative numbers need to be
> >>checked. Unicode is only defined for 0-0x7fffffff, i.e., positive
> >>only, and I've addressed that. The rest is just superstition.
> >
> >You've addressed one specific issue I gave, without changing your
> >argument one bit despite it having failed before.
> >Where do you suddenly get the confidence that your way of arguing
> >should suddenly work now since it failed once?
> >I agree you are very, very, very likely right, but repeating an
> >argument that has already been shown to be insufficient as "proof"
> >does not make it a proof.
> 
> It is not an argument, or a belief, or an opinion. It is a simple
> fact. Whether I am confident or not doesn't change the facts.
> Converting a textual representation of a positive integer into
> binary will always use fewer bits than the original text. It amazes
> me that this even needs to be discussed.

The error as I see it is that you are arguing about "converting
integer to binary" when the real issue is strtoul.
Arguing about strtoul behaviour without refererring to neither
specification nor implementation is going to lead to mistakes
like forgetting the '-' case. The point of a proof is to make
it nearly impossible to forget any such cases.

> strtoul() is specified in ANSI and a bunch of other C standards. If
> you can't rely on its behavior on a given platform then probably
> none of your software will work on that platform anyway.

Would that much software break if an strtoul also accepted "~0" to mean
0xffffffff? What if locale/LC_NUMERIC is not set to C, might something
strange happen? E.g. my man page says
"In  locales  other than the "C" locale, other strings may be accepted.",
could that include a different character having the meaning of '-'?
(Note MPlayer does not support locale != "C" at least currently, so
it is not a convincing argument, I'm just trying to show you why
_I_ don't think there's anything obvious here).

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?