On 2020/02/03 21:40, Jason McIntyre wrote: > 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
The "unspecified behaviour" relates to changing the contents of the string after calling openlog. If the storage of the string does not persist (i.e. if it is freed) then it will definitely fail, in a bad way. My attempt at changing your wording to add this information back results in it turning into pretty much the same as Ingo's (but with slightly worse choice of words).
