Closed
Bug 901820
Opened 11 years ago
Closed 11 years ago
Replace nsCRT::strdup with strdup
Categories
(Core :: General, defect)
Core
General
Tracking
()
RESOLVED
FIXED
mozilla26
People
(Reporter: m_kato, Assigned: m_kato)
References
Details
Attachments
(4 files, 2 obsolete files)
21.61 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
7.95 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
1.75 KB,
patch
|
neil
:
review+
|
Details | Diff | Splinter Review |
1.36 KB,
patch
|
neil
:
review+
|
Details | Diff | Splinter Review |
Also, no one uses nsCRT::strndup, so it should be removed
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Comment 2•11 years ago
|
||
Assignee | ||
Comment 3•11 years ago
|
||
Comment on attachment 786128 [details] [diff] [review] Part 2. Remove nsCRT::strdup/nsCRT::strdup impl. ># HG changeset patch ># Parent 97e97cf477c7d772b5b9a68a6bbbecb88cfec780 > >diff --git a/intl/uconv/ucvcn/nsGBKConvUtil.cpp b/intl/uconv/ucvcn/nsGBKConvUtil.cpp >--- a/intl/uconv/ucvcn/nsGBKConvUtil.cpp >+++ b/intl/uconv/ucvcn/nsGBKConvUtil.cpp >@@ -1,16 +1,16 @@ > /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ > /* This Source Code Form is subject to the terms of the Mozilla Public > * License, v. 2.0. If a copy of the MPL was not distributed with this > * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ > > #include "nsGBKConvUtil.h" > #include "gbku.h" >-#include "nsCRT.h" >+#include "nsDebug.h" > #define MAX_GBK_LENGTH 24066 /* (0xfe-0x80)*(0xfe-0x3f) */ > //-------------------------------------------------------------------- > // nsGBKConvUtil > //-------------------------------------------------------------------- > > static const PRUnichar gGBKToUnicodeTable[MAX_GBK_LENGTH] = { > #include "cp936map.h" > }; >diff --git a/xpcom/ds/moz.build b/xpcom/ds/moz.build >--- a/xpcom/ds/moz.build >+++ b/xpcom/ds/moz.build >@@ -46,17 +46,16 @@ XPIDL_MODULE = 'xpcom_ds' > MODULE = 'xpcom' > > EXPORTS += [ > 'nsArray.h', > 'nsAtomService.h', > 'nsCRT.h', > 'nsCharSeparatedTokenizer.h', > 'nsCheapSets.h', >- 'nsCppSharedAllocator.h', > 'nsExpirationTracker.h', > 'nsHashPropertyBag.h', > 'nsHashtable.h', > 'nsIByteBuffer.h', > 'nsIUnicharBuffer.h', > 'nsMathUtils.h', > 'nsObserverList.h', > 'nsObserverService.h', >diff --git a/xpcom/ds/nsCRT.cpp b/xpcom/ds/nsCRT.cpp >--- a/xpcom/ds/nsCRT.cpp >+++ b/xpcom/ds/nsCRT.cpp >@@ -144,34 +144,16 @@ const char* nsCRT::memmem(const char* ha > for (uint32_t i = 0; i < haystackLen - needleLen; i++) { > if (!memcmp(haystack + i, needle, needleLen)) > return haystack + i; > } > #endif > return NULL; > } > >-PRUnichar* nsCRT::strdup(const PRUnichar* str) >-{ >- uint32_t len = NS_strlen(str); >- return strndup(str, len); >-} >- >-PRUnichar* nsCRT::strndup(const PRUnichar* str, uint32_t len) >-{ >- nsCppSharedAllocator<PRUnichar> shared_allocator; >- PRUnichar* rslt = shared_allocator.allocate(len + 1); // add one for the null >- // PRUnichar* rslt = new PRUnichar[len + 1]; >- >- if (rslt == NULL) return NULL; >- memcpy(rslt, str, len * sizeof(PRUnichar)); >- rslt[len] = 0; >- return rslt; >-} >- > // This should use NSPR but NSPR isn't exporting its PR_strtoll function > // Until then... > int64_t nsCRT::atoll(const char *str) > { > if (!str) > return 0; > > int64_t ll = 0; >diff --git a/xpcom/ds/nsCRT.h b/xpcom/ds/nsCRT.h >--- a/xpcom/ds/nsCRT.h >+++ b/xpcom/ds/nsCRT.h >@@ -5,17 +5,16 @@ > #ifndef nsCRT_h___ > #define nsCRT_h___ > > #include <stdlib.h> > #include <string.h> > #include <ctype.h> > #include "plstr.h" > #include "nscore.h" >-#include "nsCppSharedAllocator.h" > #include "nsCRTGlue.h" > > #if defined(XP_WIN) || defined(XP_OS2) > # define NS_LINEBREAK "\015\012" > # define NS_LINEBREAK_LEN 2 > #else > # ifdef XP_UNIX > # define NS_LINEBREAK "\012" >@@ -97,28 +96,16 @@ public: > > static int32_t strncmp(const char* s1, const char* s2, int32_t aMaxLen) { > // inline the first test (assumes strings are not null): > int32_t diff = ((const unsigned char*)s1)[0] - ((const unsigned char*)s2)[0]; > if (diff != 0) return diff; > return int32_t(PL_strncmp(s1,s2,unsigned(aMaxLen))); > } > >- static char* strdup(const char* str) { >- return PL_strdup(str); >- } >- >- static char* strndup(const char* str, uint32_t len) { >- return PL_strndup(str, len); >- } >- >- static void free(char* str) { >- PL_strfree(str); >- } >- > /** > > How to use this fancy (thread-safe) version of strtok: > > void main(void) { > printf("%s\n\nTokens:\n", string); > // Establish string and get the first token: > char* newStr; >@@ -142,29 +129,16 @@ public: > uint32_t aMaxLen); > > // The GNU libc has memmem, which is strstr except for binary data > // This is our own implementation that uses memmem on platforms > // where it's available. > static const char* memmem(const char* haystack, uint32_t haystackLen, > const char* needle, uint32_t needleLen); > >- // You must use nsCRT::free(PRUnichar*) to free memory allocated >- // by nsCRT::strdup(PRUnichar*). >- static PRUnichar* strdup(const PRUnichar* str); >- >- // You must use nsCRT::free(PRUnichar*) to free memory allocated >- // by strndup(PRUnichar*, uint32_t). >- static PRUnichar* strndup(const PRUnichar* str, uint32_t len); >- >- static void free(PRUnichar* str) { >- nsCppSharedAllocator<PRUnichar> shared_allocator; >- shared_allocator.deallocate(str, 0 /*we never new or kept the size*/); >- } >- > // String to longlong > static int64_t atoll(const char *str); > > static char ToUpper(char aChar) { return NS_ToUpper(aChar); } > static char ToLower(char aChar) { return NS_ToLower(aChar); } > > static bool IsUpper(char aChar) { return NS_IsUpper(aChar); } > static bool IsLower(char aChar) { return NS_IsLower(aChar); } >diff --git a/xpcom/ds/nsCppSharedAllocator.h b/xpcom/ds/nsCppSharedAllocator.h >deleted file mode 100644 >--- a/xpcom/ds/nsCppSharedAllocator.h >+++ /dev/null >@@ -1,104 +0,0 @@ >-/* This Source Code Form is subject to the terms of the Mozilla Public >- * License, v. 2.0. If a copy of the MPL was not distributed with this >- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ >- >-#ifndef nsCppSharedAllocator_h__ >-#define nsCppSharedAllocator_h__ >- >-#include "nsMemory.h" // for |nsMemory| >-#include <new> // to allow placement |new| >- >- >- // under MSVC shut off copious warnings about unused in-lines >-#ifdef _MSC_VER >- #pragma warning( disable: 4514 ) >-#endif >- >-#include <limits.h> >- >- >-template <class T> >-class nsCppSharedAllocator >- /* >- ...allows Standard Library containers, et al, to use our global shared >- (XP)COM-aware allocator. >- */ >- { >- public: >- typedef T value_type; >- typedef size_t size_type; >- typedef ptrdiff_t difference_type; >- >- typedef T* pointer; >- typedef const T* const_pointer; >- >- typedef T& reference; >- typedef const T& const_reference; >- >- >- >- nsCppSharedAllocator() { } >- >- ~nsCppSharedAllocator() { } >- >- >- pointer >- address( reference r ) const >- { >- return &r; >- } >- >- const_pointer >- address( const_reference r ) const >- { >- return &r; >- } >- >- pointer >- allocate( size_type n, const void* /*hint*/=0 ) >- { >- return reinterpret_cast<pointer>(nsMemory::Alloc(static_cast<uint32_t>(n*sizeof(T)))); >- } >- >- void >- deallocate( pointer p, size_type /*n*/ ) >- { >- nsMemory::Free(p); >- } >- >- void >- construct( pointer p, const T& val ) >- { >- new (p) T(val); >- } >- >- void >- destroy( pointer p ) >- { >- p->~T(); >- } >- >- size_type >- max_size() const >- { >- return ULONG_MAX / sizeof(T); >- } >- >- }; >- >- >-template <class T> >-bool >-operator==( const nsCppSharedAllocator<T>&, const nsCppSharedAllocator<T>& ) >- { >- return true; >- } >- >-template <class T> >-bool >-operator!=( const nsCppSharedAllocator<T>&, const nsCppSharedAllocator<T>& ) >- { >- return false; >- } >- >-#endif /* !defined(nsCppSharedAllocator_h__) */ >diff --git a/xpcom/glue/nsDeque.cpp b/xpcom/glue/nsDeque.cpp >--- a/xpcom/glue/nsDeque.cpp >+++ b/xpcom/glue/nsDeque.cpp >@@ -1,15 +1,16 @@ > /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ > /* This Source Code Form is subject to the terms of the Mozilla Public > * License, v. 2.0. If a copy of the MPL was not distributed with this > * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ > > #include "nsDeque.h" >-#include "nsCRT.h" >+#include "nsTraceRefcnt.h" >+#include <string.h> > #ifdef DEBUG_rickg > #include <stdio.h> > #endif > > /** > * 07/02/2001 09:17p 509,104 clangref.pdf from openwatcom's site > * Watcom C Language Reference Edition 11.0c > * page 118 of 297
Attachment #786128 -
Attachment is patch: true
Assignee | ||
Comment 4•11 years ago
|
||
Comment on attachment 786124 [details] [diff] [review] Part 1. Replace nsCRT::strdup with strdup Replace nsCRT::strdup with strdup/NS_strdup
Attachment #786124 -
Flags: review?(benjamin)
Assignee | ||
Comment 5•11 years ago
|
||
Comment on attachment 786128 [details] [diff] [review] Part 2. Remove nsCRT::strdup/nsCRT::strdup impl. Remove implementation of strdup/strndup and nsCppSharedAllocator.h
Attachment #786128 -
Flags: review?(benjamin)
Assignee | ||
Comment 6•11 years ago
|
||
Attachment #786130 -
Flags: review?
Assignee | ||
Updated•11 years ago
|
Attachment #786130 -
Flags: review? → review?(mbanner)
Comment 9•11 years ago
|
||
Comment on attachment 786130 [details] [diff] [review] Part 3. comm-central change Review of attachment 786130 [details] [diff] [review]: ----------------------------------------------------------------- Neil's got more experience in this area, and I'm cutting down on non-critical patches for the next few weeks.
Attachment #786130 -
Flags: review?(mbanner) → review?(neil)
Comment 10•11 years ago
|
||
(In reply to Makoto Kato from comment #4) > Replace nsCRT::strdup with strdup/NS_strdup What, if any, difference is there between strdup and NS_strdup, in particular for code that isn't linked into libxul, and where the deallocator is NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY?
Updated•11 years ago
|
Attachment #786124 -
Flags: review?(benjamin) → review+
Updated•11 years ago
|
Attachment #786128 -
Flags: review?(benjamin) → review+
Assignee | ||
Updated•11 years ago
|
Attachment #786130 -
Flags: review?(neil)
Assignee | ||
Comment 11•11 years ago
|
||
At least, Original code in c-c shouldn't use nsCRT::strdup(const char*) since this is XPCOM code. nsCRT::strdup(const char*) uses PL_strdup...
Attachment #786130 -
Attachment is obsolete: true
Attachment #788800 -
Flags: review?(neil)
Assignee | ||
Updated•11 years ago
|
Attachment #788800 -
Attachment is patch: true
Comment 12•11 years ago
|
||
Comment on attachment 788800 [details] [diff] [review] Bug 901820 - Part 3. comm-cetral part of replacing nsCRT::strdup I used to have an allocator mismatch tracking build once. After a few patches I managed to get it to open the browser, but I didn't dare open mail...
Attachment #788800 -
Flags: review?(neil) → review+
Assignee | ||
Comment 13•11 years ago
|
||
https://hg.mozilla.org/comm-central/rev/3dabad3a7bc9 for c-c
Assignee | ||
Comment 14•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/5cfc41cf48d9 https://hg.mozilla.org/integration/mozilla-inbound/rev/94fd01624434
Assignee: nobody → m_kato
Target Milestone: --- → mozilla26
Comment 15•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/5cfc41cf48d9 https://hg.mozilla.org/mozilla-central/rev/94fd01624434
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 16•11 years ago
|
||
Looks like you overlooked one occurrence in comm-central: ../../../../ldap/xpcom/src/nsLDAPService.cpp:905:38: error: ‘moz_strndup’ is not a member of ‘nsCRT’
Comment 17•11 years ago
|
||
So what is the correct fix for nsLDAPService.cpp? NS_strndup, PL_strndup, strndup? I'm not really familiar with this, so would be nice if someone else can help with that.
Assignee | ||
Comment 18•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #794547 -
Flags: review?(neil)
Assignee | ||
Comment 19•11 years ago
|
||
(In reply to Frank Wein [:mcsmurf] from comment #17) > So what is the correct fix for nsLDAPService.cpp? NS_strndup, PL_strndup, > strndup? I'm not really familiar with this, so would be nice if someone else > can help with that. Ah, I add additional patch. Since It is XPCOM method, we should use nsMemory/NS_Alloc allocator, not PL_strndup/strdup.
Comment 20•11 years ago
|
||
Comment on attachment 794547 [details] [diff] [review] fix for nsCRT::strndup > if (*p != '=') { ... > return NS_ERROR_UNEXPECTED; > } >- if (!(attrNameArray[index] = nsCRT::strndup(*component, len))) { >+ if (!(attrNameArray[index] = ToNewCString(nsDependentCString(*component, len)))) { I don't know why they have separate variables for len and p = component + len but you'll find that component[len] is not null and therefore you can't use a dependent string on it.
Attachment #794547 -
Flags: review?(neil) → review-
Assignee | ||
Comment 21•11 years ago
|
||
Attachment #794547 -
Attachment is obsolete: true
Attachment #794558 -
Flags: review?(neil)
Comment 22•11 years ago
|
||
Comment on attachment 794558 [details] [diff] [review] Fix bustrage for nsCRT::strndup removal v2 >+ if (!(attrNameArray[index] = (char*)NS_Alloc(len + 1))) { Nit: prefer static_cast<>. (The line is getting a bit long so you may want to split it into separate assignment and test statements.) > NS_ERROR("nsLDAPService::ParseDn: out of memory "); > ldap_value_free(dnComponents); > ldap_value_free(rdnComponents); > NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(index, attrNameArray); > return NS_ERROR_OUT_OF_MEMORY; > } >+ memcpy(attrNameArray[index], *component, len); >+ *(attrNameArray[index] + len) = '\0'; Nit: Just use attrNameArray[index][len].
Attachment #794558 -
Flags: review?(neil) → review+
Comment 23•11 years ago
|
||
https://hg.mozilla.org/comm-central/rev/dd169329db9d
You need to log in
before you can comment on or make changes to this bug.
Description
•