On Thu, Dec 10, 2009 at 02:55:30PM -0800, Alan Irwin wrote: > On 2009-12-10 22:03-0000 Andrew Ross wrote: > > > > > Alan, > > > > I notice your fix for the cppcheck detected issues makes use of goto's. In > > many > > circles use of goto is considered a real no-no. I'm not sure my fix of > > using a > > macro for checking error codes and cleaning up is any better (see recent > > fix to > > lib/nistcd/cdexpert.c), but we should perhaps have a coding policy on such > > things to go along with our style policy. > > I am glad you brought this up. > > I used to be fanatic about eliminating goto's, but then I read with interest > Linus's justification of them for the kernel. After that I noticed a few of > them in PLplot as well (to deal with this issue of cleanly getting out of > error conditions without a bunch of duplicate code, see plctrl.c and > plcont.c.) So I propagated that style in the latest commit. However, I > think your idea of using a macro instead to hide the duplicated code from > human eyes does look a lot better. > > If everyone else agrees with the macro approach, I would be willing > to replace all the goto's (which I believe are uniformly due to this error > cleanup issue) in our source code with your macro approach if you don't > beat me to it.
I agree that goto's do make for relatively neat way of handling error paths in C in the absence of a better solution. I'm no great fan of them but I'm no great fan of macros either - they can make debugging more tricky in some cases and obscure code unless careful. I did think for a fair while before implementing one to check return codes. Neither is a perfect solution, but it would be good to have some consistency. Andrew ------------------------------------------------------------------------------ Return on Information: Google Enterprise Search pays you back Get the facts. http://p.sf.net/sfu/google-dev2dev _______________________________________________ Plplot-devel mailing list Plplot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/plplot-devel
