Re: [Tarantool-patches] [PATCH v7 1/2] fiber: set diagnostics at madvise/mprotect failure

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

On 15/02/2020 07:57, Cyrill Gorcunov wrote:
> On Fri, Feb 14, 2020 at 11:27:50PM +0100, Vladislav Shpilevoy wrote:
>>>
>>> it is not about memory occupation but rather init it once instead
>>> of doing so during function prologue. It actually depends on compiler
>>> and modern ones would simply optimize this assignment.
>>
>> Sorry, I don't understand. What do you mean init it only once?
>> PROT_READ | PROT_WRITE is a constant. There is nothing to init or
>> to calculate. This is just a one constant integer, which does not
>> need to be saved into a variable before being passed to a function,
>> especially into a static variable.
> 
> It highly depends on compiler. It might be optimized and not allocated
> on the stack at all or it might be allocated and loaded in the procedure
> prologue (which will happen with optimization disabled).

This is what I don't like - dependency on whether a compiler will
optimize something or not.

>>> Actually there is one hidden idea -- R|W is initial flags the 'small'
>>> uses when allocates memory that's why I mention this variable in
>>> comment.
>>
>> You don't really need a static variable to mention its value in a
>> comment. Just say that you use READ|WRITE because this is what
>> usually can be done with normal memory.
> 
> And I'll have to modify both -- comment and code if we need
> to change the permissions. Thanks but no. We could do better
> and keep R|W in one place.

I can't imagine why would we need other flags here, but anyway if
that will happen, I am sure it will change much more than just a
comment. However, this is subjective.

> But if you bothers this approach I could just inline R|W, no problem.

Usually for similar things, when it is wanted to have a constant
expression as a single identifier, enum works well, and we use
that a lot, for the same reasons. So I propose you to consider a
enum then. Anonymous enum in fiber.c.

> Still there is a really big question remains -- what to do with
> the issue. I suspect Kostya has something different in mind. So
> for now I'm giving up cooking this series (until I'll be sure
> that I really understand the next step to walk).

Ok. Btw, I don't understand what is wrong with the current
solution.