Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: emk, Assigned: emk)

Tracking

unspecified
mozilla59
Points:
---

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Because nsCRT::strncmp behaves more like wmemcmp than wcsncmp and the current name is misleading.
(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
Posted patch Remove nsCRT::strncmp (obsolete) — Splinter Review
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
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)
Summary: Rename nsCRT::strncmp to nsCRT::memcmp or nsCRT::wmemcmp or something → Remove nsCRT::strncmp
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?
Attachment #8941056 - Flags: review?(erahm) → review+
Attachment #8941056 - Attachment is obsolete: true
Keywords: checkin-needed
Jorg, not sure if this affects comm-central but FYI.
Flags: needinfo?(jorgk)
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)
https://hg.mozilla.org/mozilla-central/rev/077e64a34f6b
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Duplicate of this bug: 1427871
You need to log in before you can comment on or make changes to this bug.