[interfacekit] Re: BString patch suggestion and some questions, too (and more...)

On 2003-10-30 at 18:19:06 [+0100], Axel Dörfler wrote:
[ 8< snip 8< ]
> Although I think the BString class should not crash under any 
> circumstance, the class is not designed to propagate any errors back. I 
> am not sure if it's a good idea to just delete the current string in case 
> it couldn't be enlarged, I am also not sure to deviate from the expected 
> behaviour without having the option to check a return value. But anyway, 
> I think it's better than have it crashing. We could throw an exception in 
> the future as well.

Yes, I think bad_alloc would be the best solution after all, but as Ingo 
has pointed out to me during BeGeistert, that would not go nice with the 
way the Be-API currently works, so it's as you say: in the future...

> Yes, that should be done in any case!
> But is:
>     if (&into == this)
>         return *this;
> correct? Shouldn't it just contain from + length after this method has 
> been called?

Yes, you are right!

On 2003-10-30 at 21:23:58 [+0100], Ingo Weinhold wrote:
> 
> On 2003-10-30 at 19:57:05 [+0100], you wrote:
>
> Reading the not exactly detailed BeBook documentation -- `LockBuffer() 
> returns a pointer to the object's string...' -- as a user I would 
> actually expect that the pointer returned by LockBuffer() contains the C 
> string represented by the object as a prefix. So, in case of the empty 
> string its first character should indeed be a '\0', IMHO.

I agree, so there is the special case that a call to LockBuffer() has to 
put the '\0' in by itself if the string had been NULL before, but that's 
simple.

> 
> > Rather than removing the strcmp() test, I would much rather see 
> > _GrowBy() initialize new space to '\0'.  Since a default-constructed 
> > BString is otherwise ready for use, it seems inconsistent that 
> > LockBuffer() can return something that isn't.  As it is, if I have

I agree that LockBuffer should always return something that is safely 
usable as a c-string, but apart from that I don't think it should fiddle 
with the buffer any more than neccessary. Most of the time, LockBuffer() is 
called in order to overwrite the buffer anyway (IMHO).

> > 
> >     BString myString("test");
> >     myString.LockBuffer(10);
> > 
> > _GrowBy() is going to leave me with "test\0xxxxx\0" (where 'x' is 
> > random cruft), which is not very hygenic. ;)  So making the suggested 
> > change will not only give predictable results for default-constructed 
> > BStrings, but just be generally more clean.  Depending on what the use 
> > cases are, it might be a good idea to apply this to _OpenAtBy() as well.
> 
> I'm not so sure, if that's a good idea. This will cause the 
> implementation to doubly touch the new space in the cases where _GrowBy() 
> and _OpenAtBy() are used, thus decreasing performance. In case of 
> LockBuffer() it will even be worse, if the allocated size is far greater 
> than the actually used one.

Yes, given that LockBuffer() *does* use _GrowBy(), and that LockBuffer() is 
the only way to get a large buffer into a BString efficiently, I think 
touching the buffer twice doesn't make much sense.

> BTW, it might be a good idea to not allocate memory, when _GrowBy() is 
> asked to create an empty string, but rather free the previously allocated 
> memory (if any) and set _privateData to NULL.

Sounds reasonable to me.

> > According to [1], realloc's failure mode is such that "if the space 
> > cannot be allocated, the object shall remain unchanged." It seems more 
> > correct that BString behave in a similar fashion, especially as no 
> > direct indication of an error is made.  Of course, since the app would 
> > be in a severe low memory situation, it will probably crash relatively 
> > quickly anyway, so it's almost an aesthetic argument. ;)  

Not neccessarily, though, the given size might just not have been 
initialized correctly (which isn't nice, but shouldn't really crash the 
app, right?)

> > In order to 
> > ease internal handling of failed allocations, however, _GrowBy(), 
> > ShrinkAtBy() and _OpenAtBy() could still return NULL.

Yes, you are right! I will try to implement that tomorrow.

cheers,
        Oliver

Other related posts: