454419 - prbool bugs in security/

Status: RESOLVED FIXED

(Core Graveyard :: Security: UI, defect)

Description

[(dormant account)]

Attachment: fixes

The only thing I'm not sure about is the rv return fix in prbool func in nsPKCS12Blob.cpp as the original's logic is weird due to rv/prbool confusion.

Comment 1

[Kai Engert (:KaiE:)]

(In reply to comment #0)
> The only thing I'm not sure about is the rv return fix in prbool func in
> nsPKCS12Blob.cpp as the original's logic is weird due to rv/prbool confusion.

I agree. It seems all active code completely ignores the return value of function handleError(). Only some disabled code tried to look at it.

I propose to change handleError() to have return type "void" and change the return statements to simply "return;"

Comment 2

[(dormant account)]

That sounds good, could you do that change? Changing return statements is easy. I'm not familiar with the code an unsure of what the logic should for places that do check handleError's return value.

Comment 3

[Kai Engert (:KaiE:)]

There are no places where the return value of handleError gets checked.

Comment 4

[(dormant account)]

Attachment: fixes

You are right, I was confusing it with some unrelated handleError

Comment 5

[(dormant account)]

Kai, I'm waiting on your review so I can commit this.

Comment 6

[Kai Engert (:KaiE:)]

Comment on attachment 337973 [details] [diff] [review]
fixes

r=kaie

Please also delete the unnecessary declaration of variable keepGoing.

Thanks a lot for the patch!

Comment 7

[(dormant account)]

pushed 3e7b639abe75