Way back yonder in September we enabled logging by default on all builds. The 
world did not end. The |--disable-logging| flag remained which in theory, does 
in fact disable logging but none of our default builds use it, and 
unsurprisingly we occasionally started breaking custom builds that attempted to 
use it.

In bug 1161238 [1] we plan to remove |--disable-logging| completely, as part of 
that effort we have removed (or are about to) every instance of |#ifdef 
PR_LOGGING| from gecko code. So no more chance of breaking builds. But also now 
it's pretty clear if you are doing things you shouldn't. I found plenty of 
examples where it was assumed PR_LOGGING being defined was a rare thing (which, 
fair enough, it used to be). I've cleaned up most of the ones I found, but if 
you're responsible for code that is using logging I suggest you did a quick 
audit once I land bug 1161238.

They often took the form of something like this:

#ifdef PR_LOGGING
   nsCString message = do_a_couple_of_queries_and_build_a_msg();
   PR_LOG(foo, PR_LOG_DEBUG, ("message = %s", message.get()));
#endif

This seems innocuous enough, it's protected by an ifdef! Unfortunately that 
ifdef is generally always true now. Fear not, there's a better way!

Wrap that sucker in a |PR_LOG_TEST|:

  if (PR_LOG_TEST(foo, PR_LOG_DEBUG)) {
    nsCString message = do_a_couple_of_queries_and_build_a_msg();
    PR_LOG(foo, PR_LOG_DEBUG, ("message = %s", message.get()));
  }

Still sketched out? Is it debug only worthy? Well lets up that to a |#ifdef 
MOZ_DEBUG|:

#ifdef MOZ_DEBUG
  if (PR_LOG_TEST(foo, PR_LOG_DEBUG)) {
    nsCString message = do_a_couple_of_queries_and_build_a_msg();
    PR_LOG(foo, PR_LOG_DEBUG, ("message = %s", message.get()));
  }
#endif

Note, something like the following is fine, it only gets evaluated if the log 
level is PR_LOG_DEBUG:

  PR_LOG(foo, PR_LOG_DEBUG, ("message = %s", 
            do_a_couple_of_queries_and_build_a_msg().get()));

In the coming months I hope to land further logging enhancements, stay tuned!

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1161238
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Reply via email to