macro

D

dspfun

Hi all!

Can you see anything wrong or "bad" with the following macro?

#define LOG(level, msg) \
openlog (__FUNCTION__, LOG_CONS | LOG_PID | LOG_NDELAY, LOG_USER);
\
syslog (level, "%s", msg);\
closelog ();

I have problems using the macro with ternary operators. I believe it
should have parantheses around it all, but I cannot get it right.

Brs,
Markus
 
D

Dr Nick

dspfun said:
Hi all!

Can you see anything wrong or "bad" with the following macro?

#define LOG(level, msg) \
openlog (__FUNCTION__, LOG_CONS | LOG_PID | LOG_NDELAY, LOG_USER);
\
syslog (level, "%s", msg);\
closelog ();

I have problems using the macro with ternary operators. I believe it
should have parantheses around it all, but I cannot get it right.

Do you mean using it inside operators - like:

b==c ? LOG(10,"hello") : LOG(2,"goodbye");

If so, then you will have problems as the macro is three statements while
the ternary operator expects a single expression on each side.

If you do mean that - why are you doing it? The macro returns no value
so you'd be better with an if. If you don't mean that, what do you mean
(hint - give an example of where it's not working).

If you do want to use it with an if you need to be careful to wrap it in
{}s. You might want to use the do{ ... } while(0) trick.
 
D

dspfun

Do you mean using it inside operators - like:

b==c ? LOG(10,"hello") : LOG(2,"goodbye");

If so, then you will have problems as the macro is three statements while
the ternary operator expects a single expression on each side.

If you do mean that - why are you doing it?  The macro returns no value
so you'd be better with an if.  If you don't mean that, what do you mean
(hint - give an example of where it's not working).

If you do want to use it with an if you need to be careful to wrap it in
{}s.  You might want to use the do{ ... } while(0) trick.

Hi, thanks for your quick reply!

Here is the function/code where it is used (slightly modified, but
still is valid):

static void
foo(Boolean internal)
internal ? TRACE_IF(8, STR("Doing internal stuff\n")) : TRACE_IF(8,
STR("Not internal stuff\n"));
...snip...


TRACE_IF is defined as:

#define TRACE_IF(GROUP,MSG) LOG(LOG_INFO, MSG)


LOG is defined as:
#define LOG(level,msg) openlog (__FUNCTION__, LOG_CONS | LOG_PID |
LOG_NDELAY, LOG_USER); syslog (level, "%s", msg);closelog ();


Resulting in:

internal ? openlog (__FUNCTION__, 0x02 | 0x01 | 0x08, (1<<3)); syslog
(6, "%s", STR("Doing internal stuff\n"));closelog (); : openlog
(__FUNCTION__, 0x02 | 0x01 | 0x08, (1<<3)); syslog (6, "%s", STR("Not
internal stuff\n"));closelog ();;


Which gives the compile error:
foo.c:737: error: expected ':' before ';' token

What would be the "proper" way to define the macro LOG above?

Brs,
Markus
 
J

Joachim Schmitz

dspfun said:
Hi, thanks for your quick reply!

Here is the function/code where it is used (slightly modified, but
still is valid):

static void
foo(Boolean internal)
internal ? TRACE_IF(8, STR("Doing internal stuff\n")) : TRACE_IF(8,
STR("Not internal stuff\n"));
..snip...


TRACE_IF is defined as:

#define TRACE_IF(GROUP,MSG) LOG(LOG_INFO, MSG)


LOG is defined as:
#define LOG(level,msg) openlog (__FUNCTION__, LOG_CONS | LOG_PID |
LOG_NDELAY, LOG_USER); syslog (level, "%s", msg);closelog ();


Resulting in:

internal ? openlog (__FUNCTION__, 0x02 | 0x01 | 0x08, (1<<3)); syslog
(6, "%s", STR("Doing internal stuff\n"));closelog (); : openlog
(__FUNCTION__, 0x02 | 0x01 | 0x08, (1<<3)); syslog (6, "%s", STR("Not
internal stuff\n"));closelog ();;


Which gives the compile error:
foo.c:737: error: expected ':' before ';' token

What would be the "proper" way to define the macro LOG above?

Try this:

#define LOG(level,msg) do {\
openlog (__FUNCTION__, LOG_CONS | LOG_PID | LOG_NDELAY, LOG_USER);\
syslog (level, "%s", msg);\
closelog (); } while (0)

Bye, Jojo
 
D

dspfun

To answer your original question:

What's wrong with the macro is that it is three statements, and you
use it in a context where only an expression is required.
Solution: Don't do that.

You probably want to use the well-known trick do create a macro that
is a single statement without semicolon like:

#define LOG (level, msg) do { whatever (); } while (0)

and get rid of that rubbish where you use a ternary operator to save
an if statement, which is just rubbish and demonstrates that someone
tries to look clever without actually being clever. Or change the
#define so that the macro is an actual expression.

Thanks for all your replies!

Brs,
Markus
 
K

Keith Thompson

dspfun said:
Can you see anything wrong or "bad" with the following macro?

#define LOG(level, msg) \
openlog (__FUNCTION__, LOG_CONS | LOG_PID | LOG_NDELAY, LOG_USER);
\
syslog (level, "%s", msg);\
closelog ();

I have problems using the macro with ternary operators. I believe it
should have parantheses around it all, but I cannot get it right.

If you want the macro to be usable *as a statement*, you
should use the do ... while (0) trick; see question 10.4
in the comp.lang.c FAQ, http://c-faq.com/. Each occurrence
of any parameter name in the macro should be in parentheses.
See <https://github.com/Keith-S-Thompson/42> for an example of what
can happen if you don't follow this rule.

If you need to to be usable *as an expression*, you can't use
do/while -- but you can use the comma operator. In addition to
parenthesizing each parameter reference, the entire definition
should be enclosed in parentheses.
 
I

Ian Collins

Hi all!

Can you see anything wrong or "bad" with the following macro?

#define LOG(level, msg) \
openlog (__FUNCTION__, LOG_CONS | LOG_PID | LOG_NDELAY, LOG_USER);
\
syslog (level, "%s", msg);\
closelog ();

I have problems using the macro with ternary operators. I believe it
should have parantheses around it all, but I cannot get it right.

Reduce the macro to a simple function call:

void log( const char* func, int level, const char* msg )
{
openlog( func, LOG_CONS | LOG_PID | LOG_NDELAY, LOG_USER );
syslog( level, "%s", msg );
closelog();
}

#define LOG(level, msg) log( __func__, level, msg )

Why do people insist on making extra work for them selves by trying to
write overly complex macros were a function will do the job?
 
P

Philip Lantz

I respectfully disagree with the suggestion to put do ... while (0)
around this macro. In my experience, a macro should always be written as
an expression if it can be. In the example from the OP, there is no
reason whatsoever to write it as a statement--all that is required to
write it as an expression is to substitute commas for the semicolons and
enclose the whole thing in parentheses.

However, I do agree with the advice to avoid the ternary operator and
use an if statement. (Unless, of course, the ternary operator is within
another macro, in which case refer to the above paragraph.)

The do ... while (0) idiom is great when the macro cannot be written as
an expression.

And one final thing: are you *sure* you want to open and close the log
for every trace message?

Philip
 
P

Philip Lantz

Ian said:
Reduce the macro to a simple function call:

void log( const char* func, int level, const char* msg )
{
openlog( func, LOG_CONS | LOG_PID | LOG_NDELAY, LOG_USER );
syslog( level, "%s", msg );
closelog();
}

#define LOG(level, msg) log( __func__, level, msg )

Why do people insist on making extra work for them selves by trying to
write overly complex macros were a function will do the job?

Oh, good point! I shoulda read the whole thread before I sent my
response.

Philip
 
N

Nick Keighley

And one final thing: are you *sure* you want to open and close the log
for every trace message?

1. you can read the logfile on a Windows system while the system is
running
2. if the system crashes you get all the trace messages

you could bundle trace messages and write them out, say, every second
 
H

Hans Vlems

1. you can read the logfile on a Windows system while the system is
running
2. if the system crashes you get all the trace messages

you could bundle trace messages and write them out, say, every second

Wouldn't a call to fflush() do the same thing?
Has fflush() possibly nasty side effects perhaps?
Hans
 
I

Ian Collins

Wouldn't a call to fflush() do the same thing?
Has fflush() possibly nasty side effects perhaps?
Hans

On Unix systems, the system log isn't a file, it's a service.
 
M

Markus Wichmann

Wouldn't a call to fflush() do the same thing?
Has fflush() possibly nasty side effects perhaps?
Hans

Well, fflush() needs a FILE*. However, openlog(), closelog() and
syslog() don't return a FILE*. They don't even return an FD.

Of course one could write subroutines to talk to /dev/log, but that
would be mostly redundant.

What bothers me here is that a field commonly used for the program name
is used for the function name.

Ciao,
Markus
 
J

Jorgen Grahn

Hi all!

Can you see anything wrong or "bad" with the following macro?

#define LOG(level, msg) \
openlog (__FUNCTION__, LOG_CONS | LOG_PID | LOG_NDELAY, LOG_USER);
\
syslog (level, "%s", msg);\
closelog ();

(These comments are strictly off-topic since syslog() is a Unix thing:)

Apart from what Ian Collins wrote, there are some related to
openlog()/syslog() themselves. One that I see is that you're not
supposed to log with the function name as the "logger identity" but
rather a process or a program name.

Also, sooner or later you will want printf-style arguments.

/Jorgen
 
J

Joe keane

You probably want to use the well-known trick do create a macro that
is a single statement without semicolon like:

#define LOG (level, msg) do { whatever (); } while (0)

and get rid of that rubbish where you use a ternary operator to save
an if statement, which is just rubbish and demonstrates that someone
tries to look clever without actually being clever.

Silly.

Of *course* you want a macro to be expression if that's easily done.

e.g.

#define JK_GET_Z(JK, Z) \
(((JK)->a & 0x1) == 0 ? \
((Z) = (JK)->b >> 2 & 0x1, 0) : JK_GET_XZ(JK, Z))

You could write

#define JK_GET_Z(JK, Z) \
do { \
if (((JK->a) & 0x1) == 0) \
(Z) = (JK)->b >> 2 & 0x1; \
else
JK_GET_XZ(JK, Z); \
} while (0)

but *why*?

#define JK_GET_XZ(JK, Z) \
(((JK)->a & 0x3) == 0 ? \
((Z) = (JK)->c >> 3 & 0x1, 0) : jk_get_xz((JK), &(Z)))

You could write

#define JK_GET_XZ(JK, Z) \
do { \
int err;
if (((JK)->a & 0x3) == 0) { \
(Z) = (JK)->c >> 3 & 0x1;
err = 0; \
} else \
err = jk_get_xz((JK), &(Z)); \
__??(err); \
} while (0)

but *why*?

[of course the first of the first and the second of the second doesn't
work at all; the first of the second works with the second of the first]

I written plenty of macros.

#define PQ_FRACK(PQ) \
(PQ_IS_X(PQ) ? PQ_FRACK_X(PQ) : \
PQ_IS_Y(PQ) ? PQ_FRACK_Y(PQ) : pq_frack_other(PQ))

#define PQ_FRACK_Y(PQ) \
(((PQ)->p & PQ_SBZ) == 0 ? ((PQ)->q |= PQ_FRB, 0) : ERR_DANG)
 
K

Keith Thompson

Silly.

Of *course* you want a macro to be expression if that's easily done.

e.g.

#define JK_GET_Z(JK, Z) \
(((JK)->a & 0x1) == 0 ? \
((Z) = (JK)->b >> 2 & 0x1, 0) : JK_GET_XZ(JK, Z))

You could write

#define JK_GET_Z(JK, Z) \
do { \
if (((JK->a) & 0x1) == 0) \
(Z) = (JK)->b >> 2 & 0x1; \
else
JK_GET_XZ(JK, Z); \
} while (0)

but *why*?

If you were writing this other than as a macro, presumably you'd use the
if/else from rather than using the conditional operator, simply because
it's more readable that way. (Well, I almost certainly would.)

Yes, if you write the macro as an expression, you can use it in more
contexts. But what if you happen to know that you're *never* going to
need to use it other than in a statement context? Why not write the
macro definition in a more readable and maintainable form, when the only
cost is the loss of some flexibility that you're never going to use
anyway? Unless it's a visible part of a general-purpose library, if you
find later that it needs to be an expression you can always go back and
change it.

[...]
 
P

Philip Lantz

Keith said:
Yes, if you write the macro as an expression, you can use it in more
contexts. But what if you happen to know that you're *never* going to
need to use it other than in a statement context? Why not write the
macro definition in a more readable and maintainable form, when the only
cost is the loss of some flexibility that you're never going to use
anyway?

I think one of the main reasons for me is that using do ... while (0) in
a macro is a hack, and I try to avoid using hacks except where
necessary, and it is (in my experience) almost never necessary.

Philip
 
K

Keith Thompson

Philip Lantz said:
I think one of the main reasons for me is that using do ... while (0) in
a macro is a hack, and I try to avoid using hacks except where
necessary, and it is (in my experience) almost never necessary.

Fair enough.

But one could argue that translating if/else to ?: and ";" to "," for
the sole purpose of making a chunk of code usable as a macro is equally
hackish.

Sure, "do...while(0)" is a hack -- but it's also an idiom that's widely
used and recognized by C programmers.
 
J

Joe keane

If you were writing this other than as a macro, presumably you'd use the
if/else from rather than using the conditional operator, simply because
it's more readable that way.

If you saw code like this:

if (x > y)
z = x;
else
z = y;

would you think it's a bit silly?
But what if you happen to know that you're *never* going to need to use
it other than in a statement context?

IMHE 'never' means about 50% of the time you will find something you
hadn't thought of.
Why not write the macro definition in a more readable and maintainable
form, when the only cost is the loss of some flexibility that you're
never going to use anyway?

It's not less readable *to me*. Admittedly, i've done this sort of
thing a lot. For a newbie it might make him woozy.

There's a place for statement macros. When it's next to impossible to
make an expression macro, or it's so horribly unreadable that it makes
*me* woozy, i don't have a problem with it.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,482
Members
44,901
Latest member
Noble71S45

Latest Threads

Top