On Mon, Feb 03, 2020 at 09:53:39PM +0000, Stuart Henderson wrote: > 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). >
heh, i see. when i said: > > it reads ok, but i can't say a great deal more than that. i was referring to my lack of knowledge about the issue! so i guess i'm fine with ingo's text. jmc
