On Mon, Feb 03, 2020 at 09:28:15PM +0100, Ingo Schwarze wrote:
> Hi,
> 
> since our manual page doesn't explain the details of how openlog(3)
> uses *ident, it seems reasonable for users to conclude that it is
> safest to neither free nor modify it.
> 
> Then again, given that in our implementation, freeing it may even
> pose a security hazard, i might seem friendly to give more details.
> 
> POSIX has the same wording as our manual page (and so do 4.4BSD-Lite2,
> FreeBSD, NetBSD, and Oracle Solaris 11).  As far as i understand,
> the wording being in POSIX implies behaviour is unspecified if the
> memory becomes invalid or if its content is changed.  That's also
> how the Linux man-pages project documents it.  I don't think
> researching history is needed for this; knowing that it's unspecified
> feels sufficient.
> 
> Given that our implementation chooses to use-after-free (as it is
> permitted to) if the memory becomes invalid, i prefer the Theo's
> strong wording "must persist" to the possibly less discouraging
> "unspecified" - foremost, we are documenting *our* implementation.
> 
> Regarding changes of the content, i consider it friendly to mention
> that it is unspecified - otherwise, people might mistakenly assume
> that our behaviour were required by POSIX.
> 
> While here, add the missing pointer to POSIX, correct HISTORY,
> drop redundant verbiage from RETURN VALUES, and garbage collect .Tn.
> Admittedly, that's more than one change in one patch, but all of
> it is fairly standard, so why waste time splitting it.
> (Still, feel free to OK only parts, of course.)
> 
> OK?
>   Ingo
> 

hi.

it reads ok, but i can't say a great deal more than that.

i don;t want to quibble about the wording, but i'll say it anyway:

- "must" reads badly. i understand this may be the exact word you want,
  but i'm not a fan

- the sentence structure is slighly unbalanced. i'll explain inline:

> 
> Index: syslog.3
> ===================================================================
> RCS file: /cvs/src/lib/libc/gen/syslog.3,v
> retrieving revision 1.35
> diff -u -r1.35 syslog.3
> --- syslog.3  30 Aug 2019 20:27:25 -0000      1.35
> +++ syslog.3  3 Feb 2020 20:16:39 -0000
> @@ -216,12 +216,17 @@
>  .Fn vsyslog .
>  The parameter
>  .Fa ident
> -is a string that will be prepended to every message.
> +points to a string that will be prepended to every message;
> +its storage must persist until
> +.Fn closelog
> +or the corresponding
> +.Fn closelog_r .
> +If the content of the string is changed, behaviour is unspecified.


so it's like this now:

        This is what ident is;
        here i append some info about not changing it.
        Now i start a new sentence to add more info about not changing it.

i guess i would write it sth like:

        The parameter
        .Fa ident
        points to a string that is prepended to every message.
        It should not be changed until either
        .Fn closelog
        or
        .Fn closelog_r
        are called,
        otherwise the behaviour is unspecified.

well, you get the gist.
jmc

> +.Pp
>  The
>  .Fa logopt
>  argument
> -is a bit field specifying logging options, which is formed by
> -.Tn OR Ns 'ing
> +is a bit field specifying logging options, which is formed by OR'ing
>  one or more of the following values:
>  .Bl -tag -width LOG_AUTHPRIV
>  .It Dv LOG_CONS
> @@ -310,18 +315,6 @@
>  .Fa syslog_data
>  structure.
>  .Sh RETURN VALUES
> -The
> -.Fn closelog ,
> -.Fn closelog_r ,
> -.Fn openlog ,
> -.Fn openlog_r ,
> -.Fn syslog ,
> -.Fn syslog_r ,
> -.Fn vsyslog ,
> -and
> -.Fn vsyslog_r
> -functions return no value.
> -.Pp
>  The routines
>  .Fn setlogmask
>  and
> @@ -349,11 +342,58 @@
>  .Sh SEE ALSO
>  .Xr logger 1 ,
>  .Xr syslogd 8
> +.Sh STANDARDS
> +The functions
> +.Fn syslog ,
> +.Fn openlog ,
> +.Fn closelog ,
> +and
> +.Fn setlogmask
> +conform to the X/Open Systems Interfaces option of
> +.St -p1003.1-2008 .
> +.Pp
> +The facilities
> +.Dv LOG_AUTHPRIV ,
> +.Dv LOG_FTP ,
> +and
> +.Dv LOG_SYSLOG ,
> +the option
> +.Dv LOG_PERROR ,
> +and the macro
> +.Fn LOG_UPTO
> +are extensions to that standard.
> +.Pp
> +The standard option
> +.Dv LOG_NOWAIT
> +is deprecated in
> +.Ox
> +and has no effect.
>  .Sh HISTORY
> -These
> -functions appeared in
> -.Bx 4.2 .
> -The reentrant functions appeared in
> +The functions
> +.Fn syslog ,
> +.Fn openlog ,
> +and
> +.Fn closelog
> +appeared in
> +.Bx 4.2 ,
> +.Fn setlogmask
> +in
> +.Bx 4.3 ,
> +and
> +.Fn vsyslog
> +in
> +.Bx 4.3 Net/1 .
> +.Pp
> +The functions
> +.Fn syslog_r ,
> +.Fn vsyslog_r ,
> +.Fn openlog_r ,
> +.Fn closelog_r ,
> +and
> +.Fn setlogmask_r
> +appeared in
> +.Bx 386 0.1
> +and have been available since
>  .Ox 3.1 .
>  .Sh CAVEATS
>  It is important never to pass a string with user-supplied data as a

Reply via email to