macro
 

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

Re: macro
 

dspfun <dspfun@hotmail.com> writes:

> 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.
--
Online waterways route planner | http://canalplan.eu
Plan trips, see photos, check facilities | http://canalplan.org.uk

Re: macro
 

On 15 Jan, 14:44, Dr Nick <3-nos...@temporary-address.org.uk> wrote:
> dspfun <dsp...@hotmail.com> writes:
> > 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.
> --
> Online waterways route planner * * * * * *|http://canalplan.eu
> Plan trips, see photos, check facilities *|http://canalplan.org.uk

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

Re: macro
 

dspfun wrote:
> On 15 Jan, 14:44, Dr Nick <3-nos...@temporary-address.org.uk> wrote:
>> dspfun <dsp...@hotmail.com> writes:
>>> 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.
>> --
>> Online waterways route planner |http://canalplan.eu
>> Plan trips, see photos, check facilities |http://canalplan.org.uk
>
> 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

Re: macro
 

On 15 Jan, 15:37, "christian.bau" <christian....@cbau.wanadoo.co.uk>
wrote:
> On Jan 15, 1:58*pm, dspfun <dsp...@hotmail.com> wrote:
>
>
>
>
>
>
>
>
>
> > On 15 Jan, 14:44, Dr Nick <3-nos...@temporary-address.org.uk> wrote:
>
> > > dspfun <dsp...@hotmail.com> writes:
> > > > 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 youmean
> > > (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 itin
> > > {}s. *You might want to use the do{ ... } while(0) trick.
> > > --
> > > Online waterways route planner * * * * * *|http://canalplan.eu
> > > Plan trips, see photos, check facilities *|http://canalplan.org.uk
>
> > 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 ();;
>
> 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

Re: macro
 

dspfun <dspfun@hotmail.com> writes:

> 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.

--
Keith Thompson (The_Other_Keith) kst-u@mib.org <http://www.ghoti.net/~kst>
Will write code for food.
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

Re: macro
 

On 01/16/12 02:29 AM, dspfun wrote:
> 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?

--
Ian Collins

Re: macro
 

dspfun wrote:
> On 15 Jan, 15:37, "christian.bau" wrote:
> > On Jan 15, 1:58*pm, dspfun wrote:
> > > On 15 Jan, 14:44, Dr Nick wrote:
> > > > dspfun <dsp...@hotmail.com> writes:
> > > > > 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 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.
> >
> > > 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 ();;
> >
> > 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.

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

Re: macro
 

Ian Collins wrote:
>
> On 01/16/12 02:29 AM, dspfun wrote:
> > 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?

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

Philip

Re: macro
 

On Jan 16, 1:30*am, Philip Lantz <p...@canterey.us> wrote:

> 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