[Mono-devel-list] [Patch] RReader not to rely on bounds checking

[Lluis Sanchez]

On dl, 2005-01-31 at 13:57 +0100, Willibald Krenn wrote:
> Lluis Sanchez schrieb:
> > The "using" fix looks OK to me, but I don't see what's wrong on relying
> > on array bound checking in this case.
> 
> Hmm, and what if the bound checking does not work? (Either by accident 
> or by intention?)

Bounds checking will always work. It is a requirement of the CLR.

> Furthermore if the abcremoval is working correctly it should be able to 
> remove the bound check completely (IMO):

I don't think abcremoval can optimize this case.

>  So we'd still have one 'cmp' 
> but no exception would be raised in case pos >= len.

this should never happen anyway if the configuration file is valid.

> 
> If you want to preserve the bound-check catch code, you have to change 
> the catch clause: Currently, the catch just _swallows_ all exceptions - 
> ranging from the expected outofbounds, over sigsegvs (if bound checking 
> does not work) to possibly outofmemory and others.
> See Applied Microsoft .NET Framework Programming (p. 418f) for Richter's 
> comment on catch clauses like this.
> This code even catches (and swallows) non-CLS exceptions...

I know what are the consequences of a catch{} clause. But I really
really think that those exceptions are not a problem in this case. And
even if one of those exceptions is thrown, the error is being reported
anyway.

> 
> So out of performance, readability and correctness reasons I vote for 
> replacing.

When I browse other people's code I often see things that I would have
implemented in a different and more elegant way (more elegant to my
opinion, of course). But I try not to touch that code unless it is
clearly buggy, unnecessary slow, uses too much memory, or it is
difficult to understand. Since that RReader code does not have any of
those issues, I keep thinking that it is not necessary to change it,
even if your solution is more elegant (BTW, I didn't write the original
code myself).

I just don't want to lose time reviewing patches that don't fix real and
reproducible bugs. But since now I've already lost that time (and even
more time writing this mail), you can commit the patch if you want.

Lluis.

> 
> Willi
> 
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list