Closed
Bug 937915
Opened 11 years ago
Closed 11 years ago
Restore DB filenames in Telemetry SlowSQL strings
Categories
(Toolkit :: Telemetry, defect)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: vladan, Assigned: vladan)
Details
(Whiteboard: [qa-])
Attachments
(1 file)
1.60 KB,
patch
|
Yoric
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
bug 896737 accidentally caused us to stop reporting the DB filenames in Telemetry slowSQL strings
Attachment #831166 -
Flags: review?(dteller)
Comment 1•11 years ago
|
||
Comment on attachment 831166 [details] [diff] [review] always append the DB name as a comment Review of attachment 831166 [details] [diff] [review]: ----------------------------------------------------------------- r=me, with the changes below ::: toolkit/components/telemetry/Telemetry.cpp @@ +2029,2 @@ > } > + sanitizedSQL.AppendPrintf(" /* %s */", dbName.BeginReading()); If I recall correctly, the chartype* returned by BeginReading() is not necessarily 0-terminated. I'd be more comfortable with a series of calls to Append(). @@ +2036,5 @@ > StoreSlowSQL(aggregate, delay, Sanitized); > } > > nsAutoCString fullSQL(sql); > + fullSQL.AppendPrintf(" /* %s */", dbName.BeginReading()); Same here.
Attachment #831166 -
Flags: review?(dteller) → review+
Assignee | ||
Comment 2•11 years ago
|
||
nsString/nsCString are guaranteed to be null-terminated, and nsAutoCString is derived from nsCString https://developer.mozilla.org/en-US/docs/Mozilla_internal_string_guide#The_Major_String_Classes I could use get() if you prefer? https://developer.mozilla.org/en-US/docs/nsAutoString#get Your call.
Assignee | ||
Updated•11 years ago
|
Flags: needinfo?(dteller)
Comment 3•11 years ago
|
||
(In reply to Vladan Djeric (:vladan) from comment #2) > nsString/nsCString are guaranteed to be null-terminated, and nsAutoCString > is derived from nsCString Well, yes, but you are using a nsACString argument which has no such guarantee. I seem to remember that you should nsPromiseFlatCString()-ify it before you call get() or BeginReading(). Which should be a no-copy if your string is a nsAutoCString.
Flags: needinfo?(dteller)
Assignee | ||
Comment 4•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/6d8762404e6f
https://hg.mozilla.org/mozilla-central/rev/6d8762404e6f
Assignee: nobody → vdjeric
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Assignee | ||
Comment 6•11 years ago
|
||
Comment on attachment 831166 [details] [diff] [review] always append the DB name as a comment [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 896737 User impact if declined: None, but Telemetry data collected by Firefox will be incomplete Testing completed (on m-c, etc.): Tested locally and on Nightly for a couple of days Risk to taking this patch (and alternatives if risky): None really String or IDL/UUID changes made by this patch: None
Attachment #831166 -
Flags: approval-mozilla-beta?
Attachment #831166 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•11 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•11 years ago
|
Attachment #831166 -
Flags: approval-mozilla-beta?
Attachment #831166 -
Flags: approval-mozilla-beta+
Attachment #831166 -
Flags: approval-mozilla-aurora?
Attachment #831166 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 7•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/07e7f637e2a4 https://hg.mozilla.org/releases/mozilla-beta/rev/158aeb4c6657
Assignee | ||
Updated•11 years ago
|
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•