I finally got around to revising the text for this section of the coding style doc to bring it inline with what I believe to be our prevailing usage. Please let me know if you have any objections. -Darin
On Wed, May 27, 2009 at 11:42 AM, Darin Fisher <da...@chromium.org> wrote: > 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 <e...@chromium.org> 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 <erik...@google.com> 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 <da...@chromium.org> >> wrote: >> >> >> >> 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 -~----------~----~----~----~------~----~------~--~---
