245673 - Named literal strings aren't const

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P2)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

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.

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

Comment 2

[Darin Fisher]

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)

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

(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)))

Comment 4

[Darin Fisher]

ok... probably nsLiteralString.h should stop referencing nsAFlatString, but that
doesn't have to happen now.  thanks for the patch.

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

checked in