Add nsAdoptingString helper class

RESOLVED FIXED

Status

()

Core
String
RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: jst, Assigned: jst)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

14 years ago
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).
(Assignee)

Comment 1

14 years ago
Created attachment 141838 [details] [diff] [review]
Implement nsAdoptingString.

This is all inline, so hardly any codesize penalties...
(Assignee)

Updated

14 years ago
Attachment #141838 - Flags: superreview?(darin)
Attachment #141838 - Flags: review?(dbaron)
(Assignee)

Comment 2

14 years ago
Created attachment 141845 [details] [diff] [review]
Updated fix based on discussion with darin.
(Assignee)

Updated

14 years ago
Attachment #141845 - Flags: superreview?(darin)
Attachment #141845 - Flags: review?(dbaron)
(Assignee)

Updated

14 years ago
Attachment #141838 - Attachment is obsolete: true
Attachment #141838 - Flags: superreview?(darin)
Attachment #141838 - Flags: review?(dbaron)
(Assignee)

Comment 3

14 years ago
The comment above nsTAdoptString in the patch talks about the wrong string
classes. Fixed locally.
Status: NEW → ASSIGNED
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.

r=dbaron
Attachment #141845 - Flags: review?(dbaron) → review+

Updated

14 years ago
Attachment #141845 - Flags: superreview?(darin) → superreview+
(Assignee)

Comment 5

14 years ago
Fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 14 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?
(Assignee)

Comment 7

14 years ago
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.
You need to log in before you can comment on or make changes to this bug.