errorchecking branch - ode-users

Message from discussion errorchecking branch

Oleh Derevenko

More options Mar 22, 11:10 am

From: "Oleh Derevenko" <o...@eleks.lviv.ua>

Date: Sat, 22 Mar 2008 17:10:31 +0200

Local: Sat, Mar 22 2008 11:10 am

Subject: Re: [ode-users] errorchecking branch

  Reply to author
  |
  Forward
  | Print
  | View thread
  | Show original
  | Report this message
  | Find messages by this author
  

1) It is incorrect to replace ASSERT with return 0. If code does not expect
NULL-pointer it will not do any good to just return because further code in
caller may not be able to handle that invalid return.
Each such assert must be checked. You must verify all the places the
function could be called from and make sure the caller can handle that
invalid result. Do you realize what amound of work is that? That is
monotonous work that makes you tired very soon. Can you predict how many
errors you'll make?
2) ASSERT is written in all uppercase to distinguish it from ordinary
functions. It shows you immediately, that ASSERT is not a function but a
macro and its behavior may change or it may be removed from code. Changing
case to function-like makes code much more unclear.
3) Returns from inside of the function are the bad style. They lead to
potential bugs. Especially dangerous are returns hidden in macros and
especially when those macros look like functions.

My resume:
You will not achieve anything by just changing one macro to another. You
need to distinguish real asserts from false asserts and change false asserts
to error return and error handling in all the callers. But this is huge
amount of work which requires analysis of whole codebase and I doubt you are
such a maniac to do it. So, please do not change anything.

Oleh Derevenko
-- ICQ: 36361783

----- Original Message -----
From: "Daniel K. O." <danielko.lis...@gmail.com>
To: <ode-users@googlegroups.com>
Sent: 22 березня 2008 р. 16:18
Subject: [ode-users] errorchecking branch

> So I just just created the errorchecking branch, where I'm adding the
> error checking code mentioned some days ago.

> Currently I'm just replacing dAASSERT() and some dUASSERT() with
> dCheck*(), taking care to not leak anything. I'm using camelCase instead
> of ALLCAPS because it's more readable and easier to type. dAASSERT()
> (and possibly dUASSERT too) will naturally disappear, and dIASSERT()
> will be moved to the private header (error-private.h).

> There are 3 behaviors:
> * dNOERRORCHECK: as the name implies
> * dFATALERRORS: just abort (will probably change into the current
> d*ASSERT() behavior)
> * dEXCEPTIONS: throws an exception
> * default: register the error with dSetLastError(), usually returns
> either an invalid or a safe value.

> Currently only ode.cpp was updated, so I would like to ask for comments
> about the implementation. Specially about how detailed the error
> messages should be. Is it worth to have an extra descriptive string,
> from the context where the error ocurred?

> PS: As this will touch a ton of lines, I started adding modelines to the
> end of the files (currently only tested on emacs). They should work with
> mainstream editors (although most require them in the top of the files).
> Would anyone oppose modelines on the top of the file (so it will work
> with vim, kate, gedit, etc)? Does VS have anything similar?

> --
> Daniel K. O.
> "The only way to succeed is to build success yourself"

Reply to author Forward