If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

nsDependentString assertions using global history

VERIFIED FIXED in mozilla0.9.7

Status

()

Core
History: Global
P1
normal
VERIFIED FIXED
16 years ago
15 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

Trunk
mozilla0.9.7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

After the landing of bug 104651 there are a bunch of null-termination assertions
about the dependent string usage when using the history window.
Created attachment 57125 [details] [diff] [review]
patch
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla0.9.7

Comment 2

16 years ago
Comment on attachment 57125 [details] [diff] [review]
patch

r=jag
Attachment #57125 - Flags: review+
Fix checked in 2001-11-10 17:27 PDT.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 4

16 years ago
yo.. you've broken the symmetry of tokenPair and made the "AgeInDays" comparison
almost totally unreadable - can you make them both 
nsDependentSingleFragmentCSubstring please?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 5

16 years ago
+    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()
> 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.
Created attachment 57403 [details] [diff] [review]
additional patch
Oh, and that nsCAutoString is being used as PromiseFlatCString.  It may well be
faster until bug 109572 is fixed.

Comment 9

16 years ago
Comment on attachment 57403 [details] [diff] [review]
additional patch

r=jag
Attachment #57403 - Flags: review+

Comment 10

16 years ago
Comment on attachment 57403 [details] [diff] [review]
additional patch

thanks.. makes that code much more readable. sr=alecf
Attachment #57403 - Flags: superreview+

Comment 11

16 years ago
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

16 years ago
The reference keeps the object alive for the scope of the reference itself.
Checked in 2001-11-11 12:37 PDT.
Status: REOPENED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED

Comment 14

16 years ago
neat, learn something new every day. Thanks guys.
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.]
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.