Closed
Bug 1427023
Opened 8 years ago
Closed 8 years ago
Remove nsCRT::strncmp
Categories
(Core :: XPCOM, enhancement)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox59 | --- | fixed |
People
(Reporter: emk, Assigned: emk)
References
Details
Attachments
(1 file, 1 obsolete file)
13.83 KB,
patch
|
Details | Diff | Splinter Review |
Because nsCRT::strncmp behaves more like wmemcmp than wcsncmp and the current name is misleading.
Comment 1•8 years ago
|
||
(In reply to Masatoshi Kimura [:emk] from comment #0)
> Because nsCRT::strncmp behaves more like wmemcmp than wcsncmp and the
> current name is misleading.
I guess you mean the char16_t* version [1], perhaps we should fix it to handle null-terminators instead of renaming?
[1] https://searchfox.org/mozilla-central/rev/51cd1093c5c2113e7b041e8cc5c9bf2186abda13/xpcom/ds/nsCRT.h#105-106
Assignee | ||
Comment 2•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•8 years ago
|
||
Comment on attachment 8941056 [details] [diff] [review]
Remove nsCRT::strncmp
I completely removed nsCRT::strncmp instead.
The char* version can be replaced with the plain strncmp.
The char16_t* version can be replaced with NS_strncmp. I confirmed that null check is not required or already done.
Attachment #8941056 -
Flags: review?(erahm)
Assignee | ||
Updated•8 years ago
|
Summary: Rename nsCRT::strncmp to nsCRT::memcmp or nsCRT::wmemcmp or something → Remove nsCRT::strncmp
Comment 4•8 years ago
|
||
Comment on attachment 8941056 [details] [diff] [review]
Remove nsCRT::strncmp
Review of attachment 8941056 [details] [diff] [review]:
-----------------------------------------------------------------
r=me but please delete the rest of the dead code :) Also it looks like we can close bug 1427871 as a dup of this now.
::: netwerk/cache/nsMemoryCacheDevice.cpp
@@ +462,5 @@
> {
> const char * clientID = static_cast<ClientIDArgs*>(args)->clientID;
> uint32_t prefixLength = static_cast<ClientIDArgs*>(args)->prefixLength;
> const char * key = entry->Key()->get();
> + return !clientID || strncmp(clientID, key, prefixLength) == 0;
Is it possible for `key` to be null here? It looks like `entry->Key()` is an `nsCString` so we should be good. We could probably change this to use `StringBeginsWith` but it's fine if you want to keep it as-is.
::: xpcom/ds/nsCRT.h
@@ -102,5 @@
> /// Like strcmp except for ucs2 strings
> static int32_t strcmp(const char16_t* aStr1, const char16_t* aStr2);
> - /// Like strcmp except for ucs2 strings
> - static int32_t strncmp(const char16_t* aStr1, const char16_t* aStr2,
> - uint32_t aMaxLen);
We can also delete the two `strncmp(const char*, const char*, ...)` implementations as well right?
Updated•8 years ago
|
Attachment #8941056 -
Flags: review?(erahm) → review+
Assignee | ||
Comment 5•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Attachment #8941056 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/077e64a34f6b
Remove nsCRT::strncmp. r=erahm
Keywords: checkin-needed
Comment 8•8 years ago
|
||
Thanks, I can't see nsCRT::strncmp in comm-central, but we'll notice it if it breaks ;-) The fix is straight forward anyway.
Flags: needinfo?(jorgk)
Comment 9•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in
before you can comment on or make changes to this bug.
Description
•