Joshua Boyd wrote:
On Mon, 2009-01-26 at 09:11 -0500, Alan Conway wrote:

I'm building with g++ (GCC) 4.3.2 20081105 (Red Hat 4.3.2-7) with no problems. What does g++ --version tell you?

$ g++ --version
g++ (Ubuntu 4.3.2-1ubuntu11) 4.3.2
Copyright (C) 2008 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is
NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
PURPOSE.

The (void) cast should tell the compiler that we're ignoring the return value deliberately. The risk with your patch is that some other compiler will then warn that we have an unused variable.

For gcc at least doing:
int ret;
ret = someFunc();
avoids the unused variable warning, while:
int ret=someFunc();
causes the unused variable warning.  Other compilers could be better at
their warning analysis that G++ though.

It seems to me that ideally, the return wouldn't just be written in a
variable, it would also actually be checked, but I didn't take the time
to look at how such errors are usually handled.
Having now reviewed some other portions of the code, am I correct in
thinking that it would be most appropriate to do something like:
if (ret) throw ErrnoException("Cannot write to pipe");
with the actual string in the ErrnoException being adjusted to meet the
circumstance?


In most situations yes. In the case of the LockFile destructor no - throwing exceptions in a destructor is Very Bad. What puzzles me is that we're using the same compiler version and I don't get the warning. For the destructor you could do this:

if (ret) QPID_LOG(warn, "Cannot unlock lock file " << path << ": " << strError(errno));

Cheers,
Alan.

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:[email protected]

Reply via email to