Remove nsCRT::memmem

RESOLVED FIXED in Firefox 61

Status

()

P3
normal
RESOLVED FIXED
a year ago
11 months ago

People

(Reporter: emk, Assigned: vpit3833)

Tracking

({good-first-bug})

unspecified
mozilla61
good-first-bug
Points:
---

Firefox Tracking Flags

(firefox61 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

a year ago
This is not used anymore (in both m-c and c-c).
Keywords: good-first-bug
(Assignee)

Comment 1

a year ago
Posted patch bug-1427025-attempt-01.patch (obsolete) — Splinter Review
I like to work on this bug.  Based on the understanding of the title and initial comment, I am attaching this patch.  Please comment.
Flags: needinfo?(nfroyd)
Attachment #8966070 - Flags: review?(nfroyd)
Comment on attachment 8966070 [details] [diff] [review]
bug-1427025-attempt-01.patch

This patch looks good, thank you!  There's no need to request review on a patch and also request needinfo from the same person, though; just requesting review on the patch is enough.
Flags: needinfo?(nfroyd)
Attachment #8966070 - Flags: review?(nfroyd) → review+
Assignee: nobody → venkateshpitta
Keywords: checkin-needed
OS: Unspecified → All
Priority: -- → P3
Hardware: Unspecified → All
Thanks for the patch, Venkatesh! Before we land it, can you please attach an updated patch which includes proper commit information (name, email, and commit message)? After that, feel free to set checkin-needed on the bug again. Thanks!
Flags: needinfo?(venkateshpitta)
Keywords: checkin-needed
(Assignee)

Comment 4

a year ago
Attachment #8966070 - Attachment is obsolete: true
Flags: needinfo?(venkateshpitta)
Attachment #8966331 - Flags: checkin?(ryanvm)
Comment on attachment 8966331 [details] [diff] [review]
bug-1427025-attempt-02.patch

I'm not the person who reviewed it, but we can fix that on checkin :)
Attachment #8966331 - Flags: checkin?(ryanvm)
Keywords: checkin-needed

Comment 6

a year ago
Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9bbfe8e4ba8
Remove nsCRT::memmem. r=froydnj
Keywords: checkin-needed

Comment 7

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e9bbfe8e4ba8
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox61: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
(Assignee)

Comment 9

11 months ago
Can this patch be submitted as part of the same bug?
Attachment #8966889 - Flags: checkin?(nfroyd)
Comment on attachment 8966889 [details] [diff] [review]
bug-1427025-part-02.patch

Please file a different bug for this change.

This patch is also incorrect: the check for dladdr is being removed as well, and HAVE_DLADDR is used in several places:

https://searchfox.org/mozilla-central/search?q=HAVE_DLADDR&path=

Removing the " memmem" string from that line would be the correct thing to do.
Attachment #8966889 - Flags: checkin?(nfroyd)
You need to log in before you can comment on or make changes to this bug.