Bug 1588260 part 1. Make some XSLT comparator code a bit more type-safe. r=peterv.
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 11 Nov 2019 18:03:34 +0000
changeset 501488 dfa02c90256c40834b0db938ce769506dc2955da
parent 501487 56036a1ba7814f521e0027e1857d1639d2810827
child 501489 d9acb8da07684d0511722574cb8a673322604f00
push id36794
push useraciure@mozilla.com
push dateTue, 12 Nov 2019 09:43:07 +0000
treeherdermozilla-central@5f0b392beadb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs1588260
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1588260 part 1. Make some XSLT comparator code a bit more type-safe. r=peterv. Differential Revision: https://phabricator.services.mozilla.com/D49051
dom/xslt/xslt/txXPathResultComparator.cpp
dom/xslt/xslt/txXPathResultComparator.h
--- a/dom/xslt/xslt/txXPathResultComparator.cpp
+++ b/dom/xslt/xslt/txXPathResultComparator.cpp
@@ -48,18 +48,18 @@ nsresult txResultStringComparator::creat
                                                        txObject*& aResult) {
   nsAutoPtr<StringValue> val(new StringValue);
   if (!val) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   if (!mCollation) return NS_ERROR_FAILURE;
 
-  val->mCaseKey = new nsString;
-  nsString& nsCaseKey = *(nsString*)val->mCaseKey;
+  val->mCaseKeyString = new nsString;
+  nsString& nsCaseKey = *val->mCaseKeyString;
   nsresult rv = aExpr->evaluateToString(aContext, nsCaseKey);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (nsCaseKey.IsEmpty()) {
     aResult = val.forget();
 
     return NS_OK;
   }
@@ -92,63 +92,68 @@ int txResultStringComparator::compareVal
                                      strval2->mKey, strval2->mLength, &result);
   if (NS_FAILED(rv)) {
     // XXX ErrorReport
     return -1;
   }
 
   if (result != 0) return ((mSorting & kAscending) ? 1 : -1) * result;
 
-  if ((strval1->mCaseLength == 0) && (strval1->mLength != 0)) {
-    nsString* caseString = (nsString*)strval1->mCaseKey;
-    rv = mCollation->AllocateRawSortKey(
-        nsICollation::kCollationCaseSensitive, *caseString,
-        (uint8_t**)&strval1->mCaseKey, &strval1->mCaseLength);
+  if ((strval1->mCaseKeyLength == 0) && (strval1->mLength != 0)) {
+    rv = strval1->initCaseKeyBuffer(mCollation);
     if (NS_FAILED(rv)) {
       // XXX ErrorReport
-      strval1->mCaseKey = caseString;
-      strval1->mCaseLength = 0;
       return -1;
     }
-    delete caseString;
   }
-  if ((strval2->mCaseLength == 0) && (strval2->mLength != 0)) {
-    nsString* caseString = (nsString*)strval2->mCaseKey;
-    rv = mCollation->AllocateRawSortKey(
-        nsICollation::kCollationCaseSensitive, *caseString,
-        (uint8_t**)&strval2->mCaseKey, &strval2->mCaseLength);
+  if ((strval2->mCaseKeyLength == 0) && (strval2->mLength != 0)) {
+    rv = strval2->initCaseKeyBuffer(mCollation);
     if (NS_FAILED(rv)) {
       // XXX ErrorReport
-      strval2->mCaseKey = caseString;
-      strval2->mCaseLength = 0;
       return -1;
     }
-    delete caseString;
   }
   rv = mCollation->CompareRawSortKey(
-      (uint8_t*)strval1->mCaseKey, strval1->mCaseLength,
-      (uint8_t*)strval2->mCaseKey, strval2->mCaseLength, &result);
+      strval1->mCaseKeyBuffer, strval1->mCaseKeyLength, strval2->mCaseKeyBuffer,
+      strval2->mCaseKeyLength, &result);
   if (NS_FAILED(rv)) {
     // XXX ErrorReport
     return -1;
   }
 
   return ((mSorting & kAscending) ? 1 : -1) *
          ((mSorting & kUpperFirst) ? -1 : 1) * result;
 }
 
 txResultStringComparator::StringValue::StringValue()
-    : mKey(0), mCaseKey(0), mLength(0), mCaseLength(0) {}
+    : mKey(0), mCaseKeyString(nullptr), mLength(0), mCaseKeyLength(0) {}
 
 txResultStringComparator::StringValue::~StringValue() {
   free(mKey);
-  if (mCaseLength > 0)
-    free(mCaseKey);
+  if (mCaseKeyLength > 0)
+    free(mCaseKeyBuffer);
   else
-    delete (nsString*)mCaseKey;
+    delete mCaseKeyString;
+}
+
+nsresult txResultStringComparator::StringValue::initCaseKeyBuffer(
+    nsICollation* aCollation) {
+  nsString* caseString = mCaseKeyString;
+  mCaseKeyString = nullptr;
+  nsresult rv = aCollation->AllocateRawSortKey(
+      nsICollation::kCollationCaseSensitive, *caseString, &mCaseKeyBuffer,
+      &mCaseKeyLength);
+  if (NS_FAILED(rv)) {
+    mCaseKeyString = caseString;
+    mCaseKeyLength = 0;
+    return rv;
+  }
+
+  delete caseString;
+  return NS_OK;
 }
 
 txResultNumberComparator::txResultNumberComparator(bool aAscending) {
   mAscending = aAscending ? 1 : -1;
 }
 
 nsresult txResultNumberComparator::createSortableValue(Expr* aExpr,
                                                        txIEvalContext* aContext,
--- a/dom/xslt/xslt/txXPathResultComparator.h
+++ b/dom/xslt/xslt/txXPathResultComparator.h
@@ -54,19 +54,27 @@ class txResultStringComparator : public 
                             uint8_t** aKey, uint32_t* aLength);
   int mSorting;
 
   class StringValue : public txObject {
    public:
     StringValue();
     ~StringValue();
 
+    nsresult initCaseKeyBuffer(nsICollation* aCollation);
+
     uint8_t* mKey;
-    void* mCaseKey;
-    uint32_t mLength, mCaseLength;
+    union {
+      nsString* mCaseKeyString;
+      uint8_t* mCaseKeyBuffer;
+    };
+    // When mCaseKeyLength is nonzero, we have an mCaseKeyBuffer, which needs to
+    // be freed with free().  Otherwise we have an mCaseKeyString, which needs
+    // to have operator delete called on it.
+    uint32_t mLength, mCaseKeyLength;
   };
 };
 
 /*
  * Compare results as numbers (data-type="number")
  */
 class txResultNumberComparator : public txXPathResultComparator {
  public: