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