Patches

[Dirk Hohndel]

On Mon, May 12, 2014 at 09:50:06PM -0300, Tomaz Canabrava wrote:
> > So I changed the silly C version of Q_ASSERT to at least tell us what the
> > offending id was and to print to stderr. But I'm not sure I agree with you
> > on removing the if (!dive) checks - if compiled without DEBUG this means
> > we will dereference NULL pointers if for some reason there's a stale id
> > somewhere.
> 
> Yes, it will. and the app will crash. the Q_ASSERT ( only in debug
> mode, of course
> ) will also make the app crash right away. the idea of an assert is
> that: stop executing
> the application right away to so the developer can fix the problem.

yes, of course.

> The ID thing, we are not making up id's for the application, we are
> getting dives that
> gives us ID's, and using it. *if* we are forgetting to clean that id
> after a dive is deleted ,
> removed or anything, it's an application bug and a crash right away is
> a good way to
> let is track it down, instead of masking it with a early return of a
> function that should
> print something, but notthing appears.

I disagree. Crashing in a release build is NOT a good way to let the user
know that something is wrong. We should avoid crashes as much as humanly
possible.

> > I would have to go back through the commit history to figure out if these
> > were spots were this could conceivably happen - but in general I'm not
> > sure I like the idea that we simply crash if we get an unexpected error
> > somewhere. I'd rather figure out ways to keep going (which is what the old
> > code did).
> >
> > Can you comment on that, Tomaz?
> >
> > I took the patch and just pushed it - so we may have to go in and add the
> > null-ness checks back in.
> 
> See if my last comment makes sense :)
> I think it does, but I'm a weird one ;p

Yes, you are. I much rather have us recover gracefully and tell the user
in a message window "hey, you ran into a situation that shouldn't happen,
here is some info on what happened, don't worry if this doesn't make
sense, just open a bug report at trac.hohndel.org, cut and paste this info
an we'll take a look. for now you can just keep going." and then print out
what happened where and whatever else in useful debug info we can give
ourselves.

/D