109172 - nsDependentString assertions using global history

Status: VERIFIED FIXED

(Core Graveyard :: History: Global, defect, P1)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

After the landing of bug 104651 there are a bunch of null-termination assertions
about the dependent string usage when using the history window.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Comment 2

[jag (Peter Annema)]

Comment on attachment 57125 [details] [diff] [review]
patch

r=jag

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in 2001-11-10 17:27 PDT.

Comment 4

[Alec Flett]

yo.. you've broken the symmetry of tokenPair and made the "AgeInDays" comparison
almost totally unreadable - can you make them both 
nsDependentSingleFragmentCSubstring please?

Comment 5

[Alec Flett]

+    const char* startPtr = (const char*)yarn.mYarn_Buf;
+    rv = gRDFService->GetResource(
+            nsCAutoString(Substring(startPtr, startPtr+yarn.mYarn_Fill)).get(),
+            getter_AddRefs(resource));

This is also totally unreadable. Can't we do something to declare the Substring
further above the GetResource call? is the nsCAutoString still necessary, with
Substring()

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

> yo.. you've broken the symmetry of tokenPair and made the "AgeInDays" comparison
> almost totally unreadable - can you make them both 
> nsDependentSingleFragmentCSubstring please?

Not easily, since nsASingleFragmentString doesn't have .get(), for good reason.
 I guess I could throw str.BeginReading(iter) all over the place, but I'd rather
not.  I had really wanted to avoid using our string classes at all here, since
this code doesn't really get along with them.

> This is also totally unreadable. Can't we do something to declare the
> Substring further above the GetResource call? is the nsCAutoString still
> necessary, with Substring()

OK, I could split it out.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: additional patch

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Oh, and that nsCAutoString is being used as PromiseFlatCString.  It may well be
faster until bug 109572 is fixed.

Comment 9

[jag (Peter Annema)]

Comment on attachment 57403 [details] [diff] [review]
additional patch

r=jag

Comment 10

[Alec Flett]

Comment on attachment 57403 [details] [diff] [review]
additional patch

thanks.. makes that code much more readable. sr=alecf

Comment 11

[Alec Flett]

Now I'm just curious:
+    const nsASingleFragmentCString& tokenName =
+        Substring(token->tokenName, token->tokenName + token->tokenNameLength);

How exactly does this work? It looks like a temporary is created by Substring()
and a reference is stored by tokenName.. but doesn't the temporary go away
almost immediately, leaving tokenName pointing to possibly-freed memory? Just
curious how this string magic does its thing...

Comment 12

[jag (Peter Annema)]

The reference keeps the object alive for the scope of the reference itself.

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Checked in 2001-11-11 12:37 PDT.

Comment 14

[Alec Flett]

neat, learn something new every day. Thanks guys.

Comment 15

[sairuh (rarely reading bugmail)]

mass-verifying claudius' Fixed bugs which haven't changed since 2001.12.31.

if you think this particular bug is not fixed, please make sure of the following
before reopening:

a. retest with a *recent* trunk build.
b. query bugzilla to see if there's an existing, open bug (new, reopened,
assigned) that covers your issue.
c. if this does need to be reopened, make sure there are specific steps to
reproduce (unless already provided and up-to-date).

thanks!

[set your search string in mail to "AmbassadorKoshNaranek" to filter out these
messages.]