Re: [PATCH] manual: Document mprotect and introduce section on memory protection

[Rical Jasan <ricaljasan at pacific dot net>]

On 11/18/2017 09:04 AM, Florian Weimer wrote:
> * Rical Jasan:
>> Given the is-or-isn't nature of "[non-]anonymous", that could work.  If
>> "file-based" is equivalent to "non-anonymous" and we stuck with the
>> latter, that'd be fine, but my concern is the (lack of) introduction of
>> these terms.  "Anonymous mapping" only appears once in the manual, isn't
>> introduced with an @dfn, and doesn't have an @cindex to find it by; and
>> this is the first occurrence of "file-based mapping".
>>
>> I can submit a patch that introduces and indexes the anonymous mapping
>> concept where it's mentioned in llio.texi, but I don't know where we
>> would provide a similarly cursory mention of file-based mappings.
>> Ultimately, I'd rather see this patch go in than get stalled on my own
>> unfamiliarity with the vernacular, and if "file-based" is how you as the
>> author would naturally refer to it, that's probably best.
> 
> We could document the MAP_FILE flag used by some systems, where a call
> to mmap must either specify MAP_FILE or MAP_ANONYMOUS (or its older
> name, MAP_ANON).

That sounds good.  I can take a stab at it with the anonymous mappings
indexing.

>>> -@vindex PROT_READ
>>> -@vindex PROT_WRITE
>>> -@vindex PROT_EXEC
>>>  @var{protect} contains flags that control what kind of access is
>>>  permitted.  They include @code{PROT_READ}, @code{PROT_WRITE}, and
>>> -@code{PROT_EXEC}, which permit reading, writing, and execution,
>>> -respectively.  Inappropriate access will cause a segfault (@pxref{Program
>>> -Error Signals}).
>>> -
>>> -Note that most hardware designs cannot support write permission without
>>> -read permission, and many do not distinguish read and execute permission.
>>> -Thus, you may receive wider permissions than you ask for, and mappings of
>>> -write-only files may be denied even if you do not use @code{PROT_READ}.
>>
>> Coming back to this now, is there a reason not to leave this paragraph in?
> 
> It's redundant with the new section on memory protection.

OK.

>>> +When a page is mapped using @code{mmap}, page protection flags can be
>>> +specified using the protection flags argument.  @xref{Memory-mapped
>>> +I/O}.
>>> +
>>> +The following flags are available:
>>> +
>>> +@vtable @code
>>> +@item PROT_WRITE
>>> +@standards{POSIX, sys/mman.h}
>>
>> I skipped over this before since we haven't canonicalized standards yet,
>> but can you provide a specific POSIX version (e.g., POSIX.1-2001)?
> 
> I can't check easily.  I don't have the paper standards, and I don't
> know where the only versions were changed retroactively.

OK.  I can't check right now either, but I don't think it should stop
the patch from going in.

>>> +@var{protection} is a combination of the @code{PROT_*} flags described
>>> +above.
>>> +
>>
>> The return values of the function are also missing:
>>
>> "@code{mprotect} returns @code{0} on success or @code{-1} on error."
> 
> Do we use @code{-1} or @math{-1}?  Existing practice is incosistent.

@math is for mathematical expressions, and the Texinfo manual says: "In
essence, @math switches into plain TeX math mode." [1]  While the
Texinfo manual never explicitly states return values should be formatted
with @code, consider the example they use for how @code looks when it's
rendered: "The function returns @code{nil}." [2]

> Do you think this can go in as-is (with this and the return value
> documentation change)?

I do.  Thank you for taking the time to write it.

> PS: It is easier for me to response from my work mail account if you
> trim your response and leave out irrelevant parts.

Sorry about that.  I usually trim a fair amount, but was going back and
forth a lot in the message, so it was hard to remove the context.  That
was a lot of uncommented-on text in the beginning I should have removed,
though.

Thanks for putting up with the review!

Rical

[1]
https://www.gnu.org/software/texinfo/manual/texinfo/html_node/Inserting-Math.html
[2]
https://www.gnu.org/software/texinfo/manual/texinfo/html_node/_0040code.html