[CRIU] [PATCH 06/11] pstree: make equal_pid handle sid comparison between nested pidnses

[Kirill Tkhai]

On 29.05.2017 14:43, Pavel Tikhomirov wrote:
> 
> 
> On 05/29/2017 02:20 PM, Dmitry Safonov wrote:
>> 2017-05-26 20:02 GMT+03:00 Pavel Tikhomirov <ptikhomirov at virtuozzo.com>:
>>> If process belonging to some session is in different pidns than leader
>>> of these session, it will have zeroes on all aditional levels in sid,
>>> so though levels for these process and leader does not match - sids do.
>>>
>>> Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
>>> ---
>>>   criu/include/pid.h | 34 ++++++++++++++++++++--------------
>>>   1 file changed, 20 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/criu/include/pid.h b/criu/include/pid.h
>>> index 72f3636..b25621a 100644
>>> --- a/criu/include/pid.h
>>> +++ b/criu/include/pid.h
>>> @@ -46,20 +46,26 @@ struct pid {
>>>          } ns[1]; /* Must be at the end of struct pid */
>>>   };
>>>
>>> -#define equal_pid(a, b)                                                        \
>>> -({                                                                     \
>>> -       int ___i, ___ret = true;                                        \
>>> -       if (a->level == b->level) {                                     \
>>> -               for (___i = 0; ___i < a->level; ___i++)                 \
>>> -                       if (a->ns[___i].virt != b->ns[___i].virt) {     \
>>> -                               ___ret = false;                         \
>>> -                               ___i = a->level; /* break */            \
>>> -                       }                                               \
>>> -       } else {                                                        \
>>> -               pr_err("Wrong pid nesting level\n");                    \
>>> -               ___ret = false;                                         \
>>> -       }                                                               \
>>> -       ___ret;                                                         \
>>> +#define equal_pid(a, b)                                                                \
>>> +({                                                                             \
>>> +       int ___i, ___ret = true;                                                \
>>> +       struct pid *___a = a, *___b = b;                                        \
>>> +       if (a->level > b->level) {                                              \
>>> +               ___a = b;                                                       \
>>> +               ___b = a;                                                       \
>>> +               }                                                               \
>>> +       for (___i = 0; ___i < ___b->level; ___i++)                              \
>>> +               if (___i < ___a->level) {                                       \
>>> +                       if (___a->ns[___i].virt != ___b->ns[___i].virt) {       \
>>> +                               ___ret = false;                                 \
>>> +                               ___i = ___b->level; /* break */                 \
>>> +                               }                                               \
>>> +                       }                                                       \
>>> +               else if (___b->ns[___i].virt != 0) {                            \
>>> +                       ___ret = false;                                         \
>>> +                       ___i = ___b->level; /* break */                         \
>>> +                       }                                                       \
>>> +       ___ret;                                                                 \
>>>   })
>>
>> I almost wish we hadn't gone down that macros-hole - and yet
>> and yet - it's rather curious, you know, this sort of life!
>>
>> Is there any reason for it not to be a static inline function?
> 
> I can see any, I left it as a macros as it was the macros before :)

The reason is simple: there was a pr_err() used to print __LINE__.
So, a caller of equal_pid() does not need to use additional print
in error case.

Now you don't have, and it may be safely converted to function.