Re: [Gnustep-cvs] Minor fix for strings initialised with data they don't own ... consistent [...]

[Richard Frith-Macdonald]

On Sunday, November 2, 2003, at 01:44 AM, Alexander Malmberg wrote:

> > + Sat Nov 01 11:10:00 2003  Richard Frith-Macdonald <address@hidden>
> > +
> > +       * Source/NSString.m: ([-copyWithZone:]) always do true copy
> > +       * Source/GSString.m: Where the string data pointed to is not
> > +       owned by the string instance, have copy operations create new
> > +       instances rather than retaining the receiver.
>
> This broke -gui pretty badly.

How did that happen?  Changing copy operations to be true copies
in some additional cases would not appear to have potential to
break anything.  Certainly I tried running quite a few gui apps without
trouble before committing.

When looking at fixing it, I found that it
> couldn't be fixed cleanly. Stepping back a bit, it became apparent that
> this is due to a design flaw in the class hierarchy under GSString. For
> now, I've committed a temporary workaround, so -gui should mostly work.
>
> The problem with GSString et al is that there are two properties of
> concrete classes in the hierarchy, which I'll call "structure" (8-bit
> data vs. unicode), and "memory management" (allocated buffers vs. 
> inline
> vs. substrings ...), and these are mixed in the class hierarchy (in 
> this
> case, "memory management" methods are leaking to subclasses with
> incompatible "memory management").

Yes ... the copy change was intended as the first of a series of fixes
for the case where strings (other than substrings) are initialised with
data they don't own, substrings are not a problem, as they retain
retain their parents and thus, indirectly, the memory ownership.
In practice, I don't think any existing code runs into the problem I
was starting to fix.  However, the problem I was fixing doesn't
appear to be the same as the problem you are looking at.

> Apart from breaking all kinds of
> design rules, this makes it harder to understand the classes, and thus
> harder to keep it working, and it makes it easy to add latent bugs.

Anyway, my plan is to remove all memory management related methods from
> GSString, GSCString, and GSUnicodeString and make then clean, abstract
> 'structure' classes. Memory management will be defined by leaf classes
> (the concrete classes), so there'll be an extra pair of subclasses of
> GSCString and GSUnicodeString that use external buffers (that may or 
> may
> not be owned by the instance; the memory management code currently in
> GS{,C,Unicode}String will end up here).

I think you are right that the memory management would be better 
separated
out into leaf classes.

> Since the 'structure' parts' interest in the memory management can be
> summed up in a single simple "is my buffer guaranteed to be valid at
> least until I am released"-flag (equivalent to the current 'free'-flag
> for the external buffer case), this will work out nicely and should fix
> this and any other bugs caused by leaking non-owned buffers.

That's not what the 'free' flag means.  The flag means 'should I free 
my buffer'
when I'm deallocated.  The data is *always* assumed to be
"guaranteed to be valid at least until I am released"
The problem is that the existing code assumes that the buffer will be 
valid
until the instance is deallocated (which is true within the gnustep 
libraries,
but might not be true for code ported from MacOS-X).
In additions to changing copy behavior to cope with this, we also need 
to
change creation of substrings so that we don't use the substring classes
for substrings of classes which don't own their buffers.