550892 - unreachable code in nsCrypto::SignText

Status: RESOLVED WORKSFORME

(Core Graveyard :: Security: UI, defect)

Description

[timeless]

2569     for (i = 2; i < argc; ++i) {
2570       JSString *caName = JS_ValueToString(cx, argv[i]);
2571       NS_ENSURE_TRUE(caName, NS_ERROR_OUT_OF_MEMORY);

after this point, caName must be non null

2574       if (!caName) {

so this code isn't reachable

2575         aResult.Append(internalError);
2576 
2577         return NS_OK;
2578       }

Comment 1

[timeless]

Attachment: proposal

Comment 2

[Kai Engert [:KaiE:]]

I believe your patch changes semantic, we'll no longer return "out of memory", but an error string (and isn't there a chance that appending an error string will fail, given we're already out of memory?).

I'd rather prefer:


     PRUint32 i;
     for (i = 2; i < argc; ++i) {
       JSString *caName = JS_ValueToString(cx, argv[i]);
       NS_ENSURE_TRUE(caName, NS_ERROR_OUT_OF_MEMORY);
-      argv[i] = STRING_TO_JSVAL(caName);

-      if (!caName) {
-        aResult.Append(internalError);
- 
-        return NS_OK;
-      }

+      argv[i] = STRING_TO_JSVAL(caName);
       caNames[i - 2] = JS_GetStringBytes(caName);
     }
 

Do you agree?

Comment 3

[Kai Engert [:KaiE:]]

Comment on attachment 431178 [details] [diff] [review]
proposal

r-, please see my comment

Comment 4

[timeless]

bug 610198 fixed this