Thanks for trying.... I want to amend what you wrote since it doesn't really capture the common usage of CHECK to help track down crashers, etc.
Also, the statement that we should always recover from a failed DCHECK seems very wrong to me. I don't think it is a good idea to try to recover from all manner of bad input to a function, but I do think it is valuable to DCHECK bad input to a function. The DCHECK helps developers when they are writing new code, but does nothing to help us know if the failure is reached in the field. Trying to recover in the field to a DCHECK failure is often difficult. You can easily suppress a crash, but the result is often that a feature just stops working. I'd much rather we get a crash report about such a condition than have no information reported at all. I think over checking inputs at runtime to functions can lead to this problem. Sure, you didn't crash, but you left the product in a gimp state, and no one but the user is aware of it. The fear of crashing is overblown. Information about bad states is far more valuable. I'd much rather read blog comments from users saying that Chrome crashes too much than hear about hangs, dead clicks, or other misbehaving non-crash conditions. -Darin On Wed, May 27, 2009 at 11:21 AM, Evan Martin <[email protected]> wrote: > I attempted to add to > http://sites.google.com/a/chromium.org/dev/developers/coding-style but > !...@#@!...@#! sites in its typically infuriating way made it so I > couldn't add text after the last bit I added -- clicking the > "unindent" button would move my cursor a paragraph up -- so I will > leave it at what I wrote, offer it up to you to improve, and take some > slow deep breaths. > > (I seriously wonder if the sites code has some "if (evan in the user > name) { go_all_crazy(); }".) > > On Wed, May 27, 2009 at 8:00 AM, Erik Kay <[email protected]> wrote: > > Perhaps someone could formalize this discussion in the style portion of > the > > chromium wiki? > > Erik > > > > On Tue, May 26, 2009 at 10:18 PM, Darin Fisher <[email protected]> > wrote: > >> > >> On Tue, May 26, 2009 at 9:13 PM, Peter Kasting <[email protected]> > >> wrote: > >>> > >>> On Tue, May 26, 2009 at 8:31 PM, Brett Wilson <[email protected]> > >>> 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: [email protected] View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
