Bug 1296231 - Fix some missing "Missing varargs cleanup" and "Resource leaks" r?froydnj draft
authorSylvestre Ledru <sledru@mozilla.com>
Thu, 18 Aug 2016 10:46:05 +0200
changeset 402481 69578c26786fc6d495824f5dcca368a115acce52
parent 399879 b8f2c13f3b125f02663b38b841a9d0b90f3b73fa
child 402548 eccc3c093a5eddf13ce898173002d923c2b94a08
child 403594 073f76b2d516c8ca5841cd52600d9a6789f0350d
child 403595 e120b896068574c15fb88b9238b8cb961432b146
push id26666
push usersledru@mozilla.com
push dateThu, 18 Aug 2016 08:50:52 +0000
reviewersfroydnj
bugs1296231
milestone51.0a1
Bug 1296231 - Fix some missing "Missing varargs cleanup" and "Resource leaks" r?froydnj MozReview-Commit-ID: AT1L351PjF6
xpcom/glue/nsTextFormatter.cpp
--- a/xpcom/glue/nsTextFormatter.cpp
+++ b/xpcom/glue/nsTextFormatter.cpp
@@ -90,16 +90,18 @@ struct NumArgState
 #define _LEFT		0x1
 #define _SIGNED		0x2
 #define _SPACED		0x4
 #define _ZEROS		0x8
 #define _NEG		0x10
 
 #define ELEMENTS_OF(array_) (sizeof(array_) / sizeof(array_[0]))
 
+#define PR_CHECK_DELETE(nas) if (nas && (nas != nasArray)) { PR_DELETE(nas); }
+
 /*
 ** Fill into the buffer using the data in src
 */
 static int
 fill2(SprintfState* aState, const char16_t* aSrc, int aSrcLen, int aWidth,
       int aFlags)
 {
   char16_t space = ' ';
@@ -798,23 +800,26 @@ BuildArgArray(const char16_t* aFmt, va_l
 
       case NumArgState::UNISTRING: (void)va_arg(aAp, char16_t*);   break;
 
       default:
         if (nas != aNasArray) {
           PR_DELETE(nas);
         }
         *aRv = -1;
+        va_end(aAp);
         return nullptr;
     }
     cn++;
   }
+  va_end(aAp);
   return nas;
 }
 
+
 /*
 ** The workhorse sprintf code.
 */
 static int
 dosprintf(SprintfState* aState, const char16_t* aFmt, va_list aAp)
 {
   char16_t c;
   int flags, width, prec, radix, type;
@@ -853,31 +858,35 @@ dosprintf(SprintfState* aState, const ch
     PR_ASSERT(0);
     return rv;
   }
 
   while ((c = *aFmt++) != 0) {
     if (c != '%') {
       rv = (*aState->stuff)(aState, aFmt - 1, 1);
       if (rv < 0) {
+        va_end(aAp);
+        PR_CHECK_DELETE(nas);
         return rv;
       }
       continue;
     }
 
     /*
     ** Gobble up the % format string. Hopefully we have handled all
     ** of the strange cases!
     */
     flags = 0;
     c = *aFmt++;
     if (c == '%') {
       /* quoting a % with %% */
       rv = (*aState->stuff)(aState, aFmt - 1, 1);
       if (rv < 0) {
+        va_end(aAp);
+        PR_CHECK_DELETE(nas);
         return rv;
       }
       continue;
     }
 
     if (nas) {
       /* the aFmt contains the Numbered Arguments feature */
       i = 0;
@@ -886,16 +895,17 @@ dosprintf(SprintfState* aState, const ch
         i = (i * 10) + (c - '0');
         c = *aFmt++;
       }
 
       if (nas[i - 1].type == NumArgState::UNKNOWN) {
         if (nas != nasArray) {
           PR_DELETE(nas);
         }
+        va_end(aAp);
         return -1;
       }
 
       VARARGS_ASSIGN(aAp, nas[i - 1].ap);
       c = *aFmt++;
     }
 
     /*
@@ -1032,32 +1042,36 @@ dosprintf(SprintfState* aState, const ch
               flags |= _NEG;
             }
             goto do_long;
           case NumArgState::UINT32:
             u.l = (long)va_arg(aAp, uint32_t);
           do_long:
             rv = cvt_l(aState, u.l, width, prec, radix, type, flags, hexp);
             if (rv < 0) {
+              va_end(aAp);
+              PR_CHECK_DELETE(nas);
               return rv;
             }
             break;
 
           case NumArgState::INT64:
             u.ll = va_arg(aAp, int64_t);
             if (u.ll < 0) {
               u.ll = -u.ll;
               flags |= _NEG;
             }
             goto do_longlong;
           case NumArgState::UINT64:
             u.ll = va_arg(aAp, uint64_t);
           do_longlong:
             rv = cvt_ll(aState, u.ll, width, prec, radix, type, flags, hexp);
             if (rv < 0) {
+              va_end(aAp);
+              PR_CHECK_DELETE(nas);
               return rv;
             }
             break;
         }
         break;
 
       case 'e':
       case 'E':
@@ -1072,28 +1086,34 @@ dosprintf(SprintfState* aState, const ch
         break;
 
       case 'c':
         u.ch = va_arg(aAp, int);
         if ((flags & _LEFT) == 0) {
           while (width-- > 1) {
             rv = (*aState->stuff)(aState, &space, 1);
             if (rv < 0) {
+              va_end(aAp);
+              PR_CHECK_DELETE(nas);
               return rv;
             }
           }
         }
         rv = (*aState->stuff)(aState, &u.ch, 1);
         if (rv < 0) {
+          va_end(aAp);
+          PR_CHECK_DELETE(nas);
           return rv;
         }
         if (flags & _LEFT) {
           while (width-- > 1) {
             rv = (*aState->stuff)(aState, &space, 1);
             if (rv < 0) {
+              va_end(aAp);
+              PR_CHECK_DELETE(nas);
               return rv;
             }
           }
         }
         break;
 
       case 'p':
         if (sizeof(void*) == sizeof(int32_t)) {
@@ -1115,24 +1135,28 @@ dosprintf(SprintfState* aState, const ch
         PR_ASSERT(0);
         break;
 #endif
 
       case 'S':
         u.S = va_arg(aAp, const char16_t*);
         rv = cvt_S(aState, u.S, width, prec, flags);
         if (rv < 0) {
+          va_end(aAp);
+          PR_CHECK_DELETE(nas);
           return rv;
         }
         break;
 
       case 's':
         u.s = va_arg(aAp, const char*);
         rv = cvt_s(aState, u.s, width, prec, flags);
         if (rv < 0) {
+          va_end(aAp);
+          PR_CHECK_DELETE(nas);
           return rv;
         }
         break;
 
       case 'n':
         u.ip = va_arg(aAp, int*);
         if (u.ip) {
           *u.ip = aState->cur - aState->base;
@@ -1142,33 +1166,36 @@ dosprintf(SprintfState* aState, const ch
       default:
         /* Not a % token after all... skip it */
 #if 0
         PR_ASSERT(0);
 #endif
         char16_t perct = '%';
         rv = (*aState->stuff)(aState, &perct, 1);
         if (rv < 0) {
+          va_end(aAp);
+          PR_CHECK_DELETE(nas);
           return rv;
         }
         rv = (*aState->stuff)(aState, aFmt - 1, 1);
         if (rv < 0) {
+          va_end(aAp);
+          PR_CHECK_DELETE(nas);
           return rv;
         }
     }
   }
 
   /* Stuff trailing NUL */
   char16_t null = '\0';
 
   rv = (*aState->stuff)(aState, &null, 1);
 
-  if (nas && (nas != nasArray)) {
-    PR_DELETE(nas);
-  }
+  va_end(aAp);
+  PR_CHECK_DELETE(nas);
 
   return rv;
 }
 
 /************************************************************************/
 
 static int
 StringStuff(SprintfState* aState, const char16_t* aStr, uint32_t aLen)