>>>>> On Fri, 27 Oct 2006 11:02:41 -0400,
      christos@zoulas.com (Christos Zoulas) said:

> | > So it is really trivial to make syslog_r async-signal-safe in our case.
> | 	:
> | > Opinions?
> | 
> | Not fine with me.
> | 
> | Your patch isn't guaranteed to work, because it still does call
> | a function which isn't guaranteed async-signal-safe.

> Which function is that?

Actually the number of functions isn't one, but 8 at least.
(I wrote the mail just after I confirmed there was at least one such
 function.)

Note that I don't mean its implementation is currently not
async-signal-safe.  I mean it's not guaranteed async-signal-safe
in the specification.  i.e. from our signal(7) man page:
     The following functions are async-signal-safe.
     Any function not listed below is unsafe to use in signal handlers.

I think one of acceptable ways is to add the following comment to the
implementations of the 8 functions:
/*
 * Although the specification does not demand that this function is
 * async-singal-safe, our syslog_r() implemetation needs it.
 */
and add the following comment to the callers of the function:
/*
 * XXX current implementation of the following functions is async-signal-safe:
 *	....
 */
Otherwise we may break the implementation by accident in future.

Another way is to define the functions as async-signal-safe in our
manpage, but I think that is overkill.


> | > Well, floating point numbers print NaN, but...

Why don't you print something like "FloatNumber" instead of "NaN"?
I think using "Nan" just confuses users.


Also, I still think using syslog_r(3) as async-signal-safe function
isn't a good idea.
I believe we should use more specific interface for such purpose.

Or, if we really will use syslog_r(3) for that purpose, at least
we should say the followings in the man page:

- It should explicitly say syslog_r() doesn't support any floating
  point formats and "%m" format.
- It should explicitly say syslog_r() doesn't support any multibyte
  codeded character set in the format string.
- The following example 
	syslog_r(LOG_INFO|LOG_LOCAL2, &sdata, "foobar error: %m");
  must be changed to:
	syslog_r(LOG_INFO|LOG_LOCAL2, &sdata, "foobar error: %s",
	    strerror(errno));
- It should say that unlike syslog(3), syslog_r(3) doesn't send
  the time of the event to syslogd(8).
- the following part must be rewritten:
    syslog_r() and the other reentrant functions should only be used where
    reentrancy is required (for instance, in a signal handler).  syslog()
    being not reentrant, only syslog_r() should be used here.  For more
    information about reentrancy and signal handlers, see signal(3).
  Because using the word "reentrant" here is confusing, because
  no other *_r() function is async-signal-safe.  It shouldn't use the
  word reentrant but async-signal-safe.

And I'd like to confirm one more thing.
I believe that you'll make only syslog_r() call snprintf_r(), and let
syslog() call snprintf() instead of snprintf_r(). Right?
If you'll make syslog() call snprintf_r() too, all third party
programs, which are currently using floating point formats, will be
broken.
-- 
soda