Closed Bug 800220 Opened 8 years ago Closed 7 years ago

Replace PL_strlen with strlen

Categories

(Core :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: m_kato, Assigned: m_kato)

References

Details

Attachments

(3 files, 1 obsolete file)

Modern C library (glibc, msvc and bionic) has optimized version of strlen.  We should use it instead of NSPR's PL_strlen.

Also, PL_strlen can return 0 without crash if the parameter is nullptr.  I need consider this if replacing.
Attached patch fix (obsolete) — Splinter Review
Attachment #733223 - Flags: review?(ehsan)
(In reply to Makoto Kato from comment #0)
> Also, PL_strlen can return 0 without crash if the parameter is nullptr.  I
> need consider this if replacing.

Does your patch here takes this into account?  It doesn't seem to after a quick look...
Comment on attachment 733223 [details] [diff] [review]
fix

Clearing the request waiting for a response to comment 2.
Attachment #733223 - Flags: review?(ehsan)
Attachment #733223 - Attachment is obsolete: true
Assignee: nobody → m_kato
Comment on attachment 740724 [details] [diff] [review]
Part 1. Replace PL_strlen with strlen

automatically replace using sed -i.
Attachment #740724 - Flags: review?(ehsan)
Comment on attachment 740726 [details] [diff] [review]
Part 2. Fix warning error

fix warning error since strlen() returns size_t.
Attachment #740726 - Flags: review?(ehsan)
Comment on attachment 740729 [details] [diff] [review]
Part 3. Add null check to avoid strlen(nullptr)

add nullptr check
Attachment #740729 - Flags: review?(ehsan)
Attachment #740726 - Flags: review?(ehsan) → review+
Comment on attachment 740729 [details] [diff] [review]
Part 3. Add null check to avoid strlen(nullptr)

Review of attachment 740729 [details] [diff] [review]:
-----------------------------------------------------------------

So are these the only places where this is needed?
Attachment #740729 - Flags: review?(ehsan) → review+
Comment on attachment 740724 [details] [diff] [review]
Part 1. Replace PL_strlen with strlen

Review of attachment 740724 [details] [diff] [review]:
-----------------------------------------------------------------

r=me if you can demonstrate that the answer to my question in comment 10 is yes.  :-)
Attachment #740724 - Flags: review?(ehsan) → review+
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #10)
> Comment on attachment 740729 [details] [diff] [review]
> Part 3. Add null check to avoid strlen(nullptr)
> 
> Review of attachment 740729 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> So are these the only places where this is needed?

Other places already have null check before strlen.

https://hg.mozilla.org/integration/mozilla-inbound/rev/308121906007
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c6a29b5d1b1
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0da12ba5361
Depends on: 952110
Depends on: 1308100
You need to log in before you can comment on or make changes to this bug.