12:16 p.m.
* Check calls to printf-like functions (and some fixes)
* Fix qemudDebug when debug not enabled
* Fix TODO so it doesn't supply a semi-colon (and all calls to this macro)
* Add vim/emacs local variables in several places
AFAICS this doesn't break the Python API build script, which was the
objection to Jim Meyerling's previous patch to do much the same
(http://www.redhat.com/archives/libvir-list/2006-March/msg00065.html).
Rich.
12:25 p.m.
On Mon, Mar 12, 2007 at 05:16:52PM +0000, Richard W.M. Jones wrote:
* Fix qemudDebug when debug not enabled
Its not clear what was broken about the existing code ?
This chunk:
#ifdef ENABLE_DEBUG
#define qemudDebug(...) qemudLog(QEMUD_DEBUG, __VA_ARGS__)
#else
-#define qemudDebug(fmt, ...) do { } while(0);
+#define qemudDebug(fmt, ...)
#endif
Will break / silently change code semantics, if qemudDebug is used in
situations like
if (foo)
qemudDebug("blah")
wizz()
Because when compiling without debug, it turns into
if (foo)
wizz()
Regards,
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
12:32 p.m.
Daniel P. Berrange wrote:
On Mon, Mar 12, 2007 at 05:16:52PM +0000, Richard W.M. Jones wrote:
> * Fix qemudDebug when debug not enabled
Its not clear what was broken about the existing code ?
This chunk:
> #ifdef ENABLE_DEBUG
> #define qemudDebug(...) qemudLog(QEMUD_DEBUG, __VA_ARGS__)
> #else
> -#define qemudDebug(fmt, ...) do { } while(0);
> +#define qemudDebug(fmt, ...)
> #endif
Will break / silently change code semantics, if qemudDebug is used in
situations like
if (foo)
qemudDebug("blah")
wizz()
I mean this code _looks_ incorrect even if it actually isn't because of
the extra ';' in the macro.
If I type this into emacs and hit [tab] a few times I get:
if (foo)
qemudDebug("blah")
wizz()
so people are at least getting what they'd expect ...
Anyway, I've just manually checked all calls to qemudDebug to see if any
omit the trailing ';', and none do.
Rich.
12:43 p.m.
On Mon, Mar 12, 2007 at 05:32:38PM +0000, Richard W.M. Jones wrote:
Daniel P. Berrange wrote:
>On Mon, Mar 12, 2007 at 05:16:52PM +0000, Richard W.M. Jones wrote:
>>* Fix qemudDebug when debug not enabled
>
>Its not clear what was broken about the existing code ?
>
>This chunk:
>
>> #ifdef ENABLE_DEBUG
>> #define qemudDebug(...) qemudLog(QEMUD_DEBUG, __VA_ARGS__)
>> #else
>>-#define qemudDebug(fmt, ...) do { } while(0);
>>+#define qemudDebug(fmt, ...)
>> #endif
>
>Will break / silently change code semantics, if qemudDebug is used in
>situations like
>
>
> if (foo)
> qemudDebug("blah")
> wizz()
I mean this code _looks_ incorrect even if it actually isn't because of
the extra ';' in the macro.
The extra ; in the macro definition is bogus. I did actually mean to
show
if (foo)
qemudDebug("blah");
Which would then turn into
if (foo)
;
Which causes GCC to complain with
cc1: warnings being treated as errors
driver.c: In function âqemudMonitorCommandâ:
driver.c:101: warning: empty body in an if-statement
If you have empty do{}while(0) then gcc won't complain.
So all we need do is drop the extra ';' from the #define for qemudDebug,
but keep the empty loop
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
12:47 p.m.
Daniel P. Berrange wrote:
Which would then turn into
if (foo)
;
Which causes GCC to complain with
cc1: warnings being treated as errors
driver.c: In function ‘qemudMonitorCommand’:
driver.c:101: warning: empty body in an if-statement
If you have empty do{}while(0) then gcc won't complain.
So all we need do is drop the extra ';' from the #define for qemudDebug,
but keep the empty loop
Ah ... what a stupid warning!
Yes agreed in that case. Can we apply with your modification then?
Rich.
10:13 p.m.
On Mon, Mar 12, 2007 at 05:47:10PM +0000, Richard W.M. Jones wrote:
Daniel P. Berrange wrote:
>Which would then turn into
>
> if (foo)
> ;
>
>Which causes GCC to complain with
>
> cc1: warnings being treated as errors
> driver.c: In function â??qemudMonitorCommandâ??:
> driver.c:101: warning: empty body in an if-statement
>
>If you have empty do{}while(0) then gcc won't complain.
>
>So all we need do is drop the extra ';' from the #define for qemudDebug,
>but keep the empty loop
Ah ... what a stupid warning!
Yes agreed in that case. Can we apply with your modification then?
Sure, that works for me.
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
7:09 a.m.
On Mon, 2007-03-12 at 17:43 +0000, Daniel P. Berrange wrote:
if (foo)
qemudDebug("blah");
Which would then turn into
if (foo)
;
Which causes GCC to complain with
cc1: warnings being treated as errors
driver.c: In function ‘qemudMonitorCommand’:
driver.c:101: warning: empty body in an if-statement
If you have empty do{}while(0) then gcc won't complain.
Yeah, that's why I added the do{}while(0) ... I shouldn't have included
the semi-colon, though.
Cheers,
Mark.
6491
days inactive
6499
days old
6 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Mark McLoughlin -
Richard W.M. Jones