Closed Bug 235041 Opened 21 years ago Closed 21 years ago

Add nsAdoptingString helper class


(Core :: XPCOM, defect)

Not set





(Reporter: jst, Assigned: jst)



(1 file, 1 obsolete file)

A helper string class that adopts alloced string buffer on construction and
frees it on destruction would make it easier to deal with code in our mixed
string object/raw string buffer world, for instance methods that return malloced
string data and expect the callers to free it, could return an nsAdoptingString
by value and leave the memory management up to the string, in stead of expecting
the callers to get it right.

Patch coming up (per irc discussion with darin).
Attached patch Implement nsAdoptingString. (obsolete) — Splinter Review
This is all inline, so hardly any codesize penalties...
Attachment #141838 - Flags: superreview?(darin)
Attachment #141838 - Flags: review?(dbaron)
Attachment #141845 - Flags: superreview?(darin)
Attachment #141845 - Flags: review?(dbaron)
Attachment #141838 - Attachment is obsolete: true
Attachment #141838 - Flags: superreview?(darin)
Attachment #141838 - Flags: review?(dbaron)
The comment above nsTAdoptString in the patch talks about the wrong string
classes. Fixed locally.
Comment on attachment 141845 [details] [diff] [review]
Updated fix based on discussion with darin.

>+      self_type& operator=( const self_type& str )
>+        {
>+          Adopt(str.mData, str.mLength);
>+          self_type& mutable_str(NS_CONST_CAST(self_type&, str));
>+          // Make str forget the buffer we just took ownership of.
>+          new (&mutable_str) self_type();

I'd prefer:

  self_type* mutable_str = NS_CONST_CAST(self_type*, &str));
  new (mutable_str) self_type();

since I dislike mixing casts and references like that.

Attachment #141845 - Flags: review?(dbaron) → review+
Attachment #141845 - Flags: superreview?(darin) → superreview+
Closed: 21 years ago
Resolution: --- → FIXED
Why not make operator= take a non-const argument? That const-cast seems
dangerous. What if someone passes an nsAdoptingString that really *is* const?
No matter where the nsAdoptingString that's assigned into an nsAdoptingString
comes from, I don't want it to not give up its buffer, even if it's const. Keep
in mind that this is not a class that's supposed to be used as a type any other
place than as a return type from functions, so what really matters is that the
copy constructor does the right thing, since that *might* be called when
returning these things by value (and the copy ctor takes a const argument, so
therefore operator= needs to take a const too, in this case).
My idea was to make the copy ctor and the operator= take a non-const argument.
Like we do in nsAutoPtr (which btw is very similar in functionality).

Unfortunatly i'm not sure if this can be made in a way that makes all compilers
happy. There are some portability issues with nsAutoPtr that would probably
appear here too.
having a copy-constructor taking non-const should prevent the compiler from
auto-generating one taking const.
Component: String → XPCOM
You need to log in before you can comment on or make changes to this bug.