Closed Bug 745863 Opened 8 years ago Closed 8 years ago

Debug logging is not compiled out in NSSBridge

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 14

People

(Reporter: gcp, Assigned: gcp)

Details

Attachments

(1 file)

Assignee: nobody → gpascutto
1) Compile out debugging if DEBUG is not set
2) No need for \n in LOG printf
3) Comment out logging of decoded passwords (shouldn't do this by default - it's no fun seeing your passwords in your logcat debug dumps)
Attachment #615416 - Flags: review?(wjohnston)
Comment on attachment 615416 [details] [diff] [review]
Patch 1. Compile out LOG statements

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

::: mozglue/android/NSSBridge.cpp
@@ -43,5 @@
>  setup_nss_functions(void *nss_handle,
>                          void *nspr_handle,
>                          void *plc_handle)
>  {
> -  __android_log_print(ANDROID_LOG_ERROR, "GeckoLibLoad", "setup nss 1");

Just remove this

@@ +48,2 @@
>    if (nss_handle == NULL || nspr_handle == NULL || plc_handle == NULL) {
> +    LOG("Missing handle");

Its actually nice to have the \n in place at times (debugging via C, which I just remembered is why that stupid printf() was there to begin with). I don't think it hurts anything to just leave it. That said, we should do it throughout if we're going to do it.

@@ +181,5 @@
>      reply.data = 0;
>      reply.len = 0;
>  
>      if (encrypt) {
> +      // LOG("Encrypting: %s", value);

I'm fine with this, but add a comment explaining why its commented out.

@@ +220,5 @@
>        (*result)[reply.len] = '\0';
>        strncpy(*result, (char *)reply.data, reply.len);
>        //asprintf(result, "%s", (char *)reply.data);
>  
> +      // LOG("Decoded %i letters: %s", reply.len, *result);

Same here.
Attachment #615416 - Flags: review?(wjohnston) → review+
https://hg.mozilla.org/mozilla-central/rev/8a1dc2f6e810
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 14
You need to log in before you can comment on or make changes to this bug.