LogInvalidCertError doesn't use about half of its arguments

RESOLVED FIXED in mozilla29

Status

()

Core
Security: PSM
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: keeler, Assigned: retornam)

Tracking

unspecified
mozilla29
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment, 3 obsolete attachments)

211 void
212 LogInvalidCertError(TransportSecurityInfo *socketInfo, 
213                     const nsACString &host, 
214                     const nsACString &hostWithPort,
215                     int32_t port,
216                     PRErrorCode errorCode,
217                     ::mozilla::psm::SSLErrorMessageType errorMessageType,
218                     nsIX509Cert* ix509)
219 {
220   nsString message;
221   socketInfo->GetErrorLogMessage(errorCode, errorMessageType, message);
222   
223   if (!message.IsEmpty()) {
224     nsContentUtils::LogSimpleConsoleError(message, "SSL");
225   }
226 }

host, hostWithPort, port, and ix509 are unused.
(Assignee)

Updated

4 years ago
Assignee: nobody → mozbugs.retornam
(Assignee)

Comment 1

4 years ago
Created attachment 8349651 [details] [diff] [review]
bug-950169.patch
Attachment #8349651 - Flags: review?(dkeeler)
Comment on attachment 8349651 [details] [diff] [review]
bug-950169.patch

Review of attachment 8349651 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. You'll also need to modify the call to LogInvalidCertError around line 423, though.

::: security/manager/ssl/src/SSLServerCertVerification.cpp
@@ +208,5 @@
>  
>  namespace {
>  
>  void
>  LogInvalidCertError(TransportSecurityInfo *socketInfo, 

While we're here, it would be nice to get rid of this unnecessary trailing whitespace.

@@ +215,4 @@
>  {
>    nsString message;
>    socketInfo->GetErrorLogMessage(errorCode, errorMessageType, message);
>    

There's some unnecessary whitespace on this line as well (although we can keep the blank line itself).
Attachment #8349651 - Flags: review?(dkeeler) → review-
(Assignee)

Comment 3

4 years ago
Created attachment 8349665 [details] [diff] [review]
bug-950169.patch

removed the extra whitespaces.
Attachment #8349651 - Attachment is obsolete: true
Attachment #8349665 - Flags: review?(dkeeler)
(Assignee)

Comment 4

4 years ago
Created attachment 8349674 [details] [diff] [review]
bug-950169.patch
Attachment #8349665 - Attachment is obsolete: true
Attachment #8349665 - Flags: review?(dkeeler)
Attachment #8349674 - Flags: review?(dkeeler)
(Assignee)

Comment 5

4 years ago
Created attachment 8349716 [details] [diff] [review]
bug-950169.patch
Attachment #8349674 - Attachment is obsolete: true
Attachment #8349674 - Flags: review?(dkeeler)
Attachment #8349716 - Flags: review?(dkeeler)
Comment on attachment 8349716 [details] [diff] [review]
bug-950169.patch

Review of attachment 8349716 [details] [diff] [review]:
-----------------------------------------------------------------

Great!
If you're not already familiar with checkin procedure, I would have a look at this: http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed
Attachment #8349716 - Flags: review?(dkeeler) → review+
https://hg.mozilla.org/mozilla-central/rev/f95bc5df7fcf
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.