Closed Bug 245673 Opened 20 years ago Closed 20 years ago

Named literal strings aren't const

Categories

(Core :: XPCOM, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla1.8alpha2

People

(Reporter: Biesinger, Assigned: Biesinger)

Details

Attachments

(1 file)

while unnamed literal strings are cast to |const nsAFlatString&|, named literal
cstrings are just nsDependentCString, non-const. this is bad since Append and
friends isn't really supported for them.
Attached patch patchSplinter Review
Assignee: string → cbiesinger
Status: NEW → ASSIGNED
Attachment #150104 - Flags: superreview?(darin)
Attachment #150104 - Flags: review?(darin)
Priority: -- → P2
Target Milestone: --- → mozilla1.8alpha2
Comment on attachment 150104 [details] [diff] [review]
patch

>Index: xpcom/string/public/nsLiteralString.h

>-#define NS_LITERAL_CSTRING(s)                     nsDependentCString(s, PRUint32(sizeof(s)-1))
>+#define NS_LITERAL_CSTRING(s)                     NS_STATIC_CAST(const nsAFlatCString&, nsDependentCString(s, PRUint32(sizeof(s)-1)))

why is this cast needed?  i think you should be able to get away
with simply prefixing the nsDependentCString with |const|, no?

also, nsAFlatCString is just a typedef for nsCSubstring, which
nsDependentCString inherits.  so, there is no need for a cast.

r+sr=darin (with this change)
Attachment #150104 - Flags: superreview?(darin)
Attachment #150104 - Flags: superreview+
Attachment #150104 - Flags: review?(darin)
Attachment #150104 - Flags: review+
(In reply to comment #2)
> why is this cast needed?  i think you should be able to get away
> with simply prefixing the nsDependentCString with |const|, no?

no, that's what I had first, the C compiler doesn't like that; that would look like:

  rv = foo->bar(const nsDependentCString("foo", 4));
which does not look like valid C++ syntax, really

> also, nsAFlatCString is just a typedef for nsCSubstring, which
> nsDependentCString inherits.  so, there is no need for a cast.

well yes. but the point of this patch is to make explicitly mark LITERAL_STRINGs
as const.
also see line 106 of this file, as it is currently:
106 #define NS_LITERAL_STRING(s)                      NS_STATIC_CAST(const
nsAFlatString&, NS_MULTILINE_LITERAL_STRING(NS_LL(s)))
ok... probably nsLiteralString.h should stop referencing nsAFlatString, but that
doesn't have to happen now.  thanks for the patch.
checked in
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Component: String → XPCOM
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: