[interfacekit] Re: more questions about BString

Oliver Tappe <openbeos@xxxxxxxxxxxxxxx> wrote:
> when implementing the BString-patch we spoke about earlier, I tried 
> to 
> follow the idea that the BString-class should clamp all input values 
> (offsets) to sensible values (0 <= offset <= length).

I don't think that this is a good idea in general - if possible, the 
call should either fail or send you to the debugger, but don't try to 
guess what you may have wanted (buggy code is buggy code).

> As the resulting testrun shows, this causes the resulting behaviour 
> to 
> differ from R5-BString in some ways:
> 
> - R5 allows calls to Insert( const char* str, int32 pos) with 
> negative 
> positions, giving IMHO weird results...
>       Insert( "INSERT THIS", -1) 
> on an empty BString yields a string that contains "NSERT THIS". 
> This behaviour ist not at all documented by the BeBook.

Does it clobber the byte at String()[-1]?

> My suggestion would be to clamp negative offsets to 0 (yielding 
> "INSERT 
> THIS" for the given example).

If it's already implemented like "NSERT THIS" I would keep it and 
update the documentation.

> - Remove( int32 fromOffset, int32 length) 
> silently accepts fromOffset<0 (using 0), but doesn't clamp length 
> likewise:
>       BString s("A String");
>       s.Remove( 4, 30);
> yields "A String" instead of "A St".

That should be fixed, yes.

> - the (I)Find...()-methods seem to always return B_ERROR if you pass 
> a
> negative fromOffset. My patched methods clamp fromOffset to 0 and 
> then
> execute the search, possibly yielding positive results.

I would keep failing, because it really doesn't make any sense to start 
searching at -1 at it's a bug in the calling code.

> No I wonder what to do with that, since the BeBook doesn't have any 
> info on 
> that...
> Should I try to stick to R5 as much as possible (be as compatible as 
> can 
> be)?
> Or is it better to try to implement consistent behaviour and clamp *
> all* 
> offset parameters?

As I said, I don't like clamping too much; it potentially hides bugs.

Bye,
   Axel.

Other related posts: