On Tue, May 26, 2009 at 9:13 PM, Peter Kasting <pkast...@chromium.org>wrote:
> On Tue, May 26, 2009 at 8:31 PM, Brett Wilson <bre...@chromium.org> wrote:
>
>> Don't bother doing an assertion when the next line will crash anyway:
>> DCHECK(foo);
>> foo->DoSomething();
>
>
> I mostly do what Brett does, but I do sometimes DCHECK in cases like this,
> where the DCHECK is placed at the very beginning of a function, and is
> really being used to indicate the function's preconditions. This is
> effectively an attempt to document in code that the developer of the
> function explicitly never wanted the function called with NULL, and if you
> fail the check, you should fix the caller rather than bandaiding with a
> conditional (which can be a tempting fix if you lack the DCHECK and just see
> someone dereference NULL). I think this adds clarity.
>
> Like Brett, I not only use CHECK for potential security problems but also
> to indicate cases that will absolutely fail later on and we should just
> catch with an easier-to-find crash now instead of a subtle bug later.
>
> PK
>
+1
It has always been the preferred convention in chrome code to not overuse
CHECK. We do however make great use of it to track down the source of
crashes. DCHECK is good for documentation and for helping developers
understand how a function should not be used.
Historically we used CHECK to protect against malformed IPC. Better to
crash than to exploit, but prior to launch we converted a lot of that code
that lived in the browser over to kill-the-renderer code
instead. So, if you are tempted to CHECK in the browser for security
reasons, step back and consider if it might be better to crash the
offending renderer that sent you
the IPC that triggered it all.
While it is good to not overuse CHECK, I think it is also reasonable to use
it when you want to know if a bad condition can happen in the field. We'll
know if the CHECK is failing in the field, and that will then guide us to
remove the CHECK and understand why our assertion was false. Of course,
there are other options like UMA logging and histograms that are probably
much better :-)
-Darin
--~--~---------~--~----~------------~-------~--~----~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~----------~----~----~----~------~----~------~--~---
