You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Joshua Boyd <jd...@etinternational.com> on 2009/01/22 17:28:55 UTC
Question about a warning (and comment about the warning)
cpp/src/qpid/sys/posix/LockFile.cpp, line 59 throws a warning when built
with g++ 4.3. Here is the compiler output:
g++ -DHAVE_CONFIG_H -I. -Igen -I./gen -Werror -pedantic -Wall -Wextra
-Wno-shadow -Wpointer-arith -Wcast-qual -Wcast-align -Wno-long-long
-Wvolatile-register-var -Winvalid-pch -Wno-system-headers
-Woverloaded-virtual -g -O2 -MT qpid/sys/posix/LockFile.lo -MD -MP -MF
qpid/sys/posix/.deps/LockFile.Tpo -c qpid/sys/posix/LockFile.cpp -fPIC
-DPIC -o qpid/sys/posix/.libs/LockFile.o
cc1plus: warnings being treated as errors
qpid/sys/posix/LockFile.cpp: In destructor
'qpid::sys::LockFile::~LockFile()':
qpid/sys/posix/LockFile.cpp:59: error: ignoring return value of 'int
lockf(int, int, __off_t)', declared with attribute warn_unused_result
qpid/sys/posix/LockFile.cpp: In destructor
'qpid::sys::LockFile::~LockFile()':
qpid/sys/posix/LockFile.cpp:59: error: ignoring return value of 'int
lockf(int, int, __off_t)', declared with attribute warn_unused_result
qpid/sys/posix/LockFile.cpp: In destructor
'qpid::sys::LockFile::~LockFile()':
qpid/sys/posix/LockFile.cpp:59: error: ignoring return value of 'int
lockf(int, int, __off_t)', declared with attribute warn_unused_result
Here is the line in question:
(void) ::lockf(f, F_ULOCK, 0); // Suppress warnings about ignoring
return value.
What seems a bit odd is that, as you can see, there is a comment about
trying to suppress that warning, but it obviously isn't working anymore.
Changing the line to:
int ret;
ret = ::lockf(f, F_ULOCK, 0); // Suppress warnings about ignoring
return value.
Does make the error go away, although it is a bit inelegant to get ret
and then not actually check if for an error condition.
In several other locations there are write calls where the return value
is not handled, which also upsets G++ 4.3.
I can send a patch of what I changed if that would help. I'm also
flipping through Jira to see if it looks appropriate to try and make an
entry there.
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: Question about a warning (and comment about the warning)
Posted by Andrew Stitcher <as...@redhat.com>.
On Mon, 2009-01-26 at 09:40 -0500, Alan Conway wrote:
> 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.
> 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:
In all probability then the issue is between distributions then not
between compilers (Ubuntu vs Red Hat/Fedora).
Of course the outputs here don't give quite enough info to know if
_exactly_ the same compiler version is being used.
Andrew
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: Question about a warning (and comment about the warning)
Posted by Alan Conway <ac...@redhat.com>.
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:dev-subscribe@qpid.apache.org
Re: Question about a warning (and comment about the warning)
Posted by Joshua Boyd <jd...@etinternational.com>.
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?
If that looks more correct, I'd be happy to adjust the patch.
Thanks for your feedback.
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: Question about a warning (and comment about the warning)
Posted by Alan Conway <ac...@redhat.com>.
Joshua Boyd wrote:
> cpp/src/qpid/sys/posix/LockFile.cpp, line 59 throws a warning when built
> with g++ 4.3. Here is the compiler output:
> g++ -DHAVE_CONFIG_H -I. -Igen -I./gen -Werror -pedantic -Wall -Wextra
> -Wno-shadow -Wpointer-arith -Wcast-qual -Wcast-align -Wno-long-long
> -Wvolatile-register-var -Winvalid-pch -Wno-system-headers
> -Woverloaded-virtual -g -O2 -MT qpid/sys/posix/LockFile.lo -MD -MP -MF
> qpid/sys/posix/.deps/LockFile.Tpo -c qpid/sys/posix/LockFile.cpp -fPIC
> -DPIC -o qpid/sys/posix/.libs/LockFile.o
> cc1plus: warnings being treated as errors
> qpid/sys/posix/LockFile.cpp: In destructor
> 'qpid::sys::LockFile::~LockFile()':
> qpid/sys/posix/LockFile.cpp:59: error: ignoring return value of 'int
> lockf(int, int, __off_t)', declared with attribute warn_unused_result
> qpid/sys/posix/LockFile.cpp: In destructor
> 'qpid::sys::LockFile::~LockFile()':
> qpid/sys/posix/LockFile.cpp:59: error: ignoring return value of 'int
> lockf(int, int, __off_t)', declared with attribute warn_unused_result
> qpid/sys/posix/LockFile.cpp: In destructor
> 'qpid::sys::LockFile::~LockFile()':
> qpid/sys/posix/LockFile.cpp:59: error: ignoring return value of 'int
> lockf(int, int, __off_t)', declared with attribute warn_unused_result
>
>
> Here is the line in question:
> (void) ::lockf(f, F_ULOCK, 0); // Suppress warnings about ignoring
> return value.
>
> What seems a bit odd is that, as you can see, there is a comment about
> trying to suppress that warning, but it obviously isn't working anymore.
>
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?
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.
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org