745863 - Debug logging is not compiled out in NSSBridge

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Gian-Carlo Pascutto [:gcp]]

Follow up for bug 740961. 
https://bugzilla.mozilla.org/show_bug.cgi?id=740961#c4

Comment 1

[Gian-Carlo Pascutto [:gcp]]

Attachment: Patch 1. Compile out LOG statements

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)

Comment 2

[Wesley Johnston (:wesj)]

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.

Comment 3

[Gian-Carlo Pascutto [:gcp]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8a1dc2f6e810

Comment 4

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/8a1dc2f6e810