On Oct 28,  1:37am, soda@sra.co.jp (SODA Noriyuki) wrote:
-- Subject: Re: syslog_r (Re: CVS commit: src/lib/libc)

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

Ah ok. I thought you meant that the implementation was not async-signal-safe.

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

If we decide to go this way, I think it is better to document them both
in the man page and the source.

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

That is easily fixed.

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

That is fine with me. This whole thing started with me trying to
port the OpenBSD libssp. I just thought that since OpenBSD uses
syslog_r as an async-signal-safe function (without it being one),
we should fix ours to be async-signal-safe.

| 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));

that would hurt, the async-signal-safeness of the example.

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

Here's a revised version of the manual:

     Aside from being reentrant, openlog_r(), closelog_r(), setlogmask_r,()
     syslog_r(), vsetlog_r() are also async-signal-safe.  Due to that fact,
     syslog_r() and vsyslog_r() have the following limitations:

     1.   The format string cannot contain multi-byte character sequences.

     2.   Floating point formats are not supported and print ``UnS''.

     3.   The time of the event is not sent to syslogd(8).

     4.   The error string in the %m format is not printed symbolically but as
          ``Error %d''.

     Because of the above limitations the reentrant versions of the syslog(3)
     functions should only be used where reentrancy or async-signal-safety is
     required.  For more information about async-signal-safe functions and
     signal handlers, see signal(7).

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

Right, syslog calls snprintf, syslog_r calls snprinf_r

christos