[CRIU] Re: [PATCH 2/3] restore: Don't close LAST_PID_PATH descriptor if it was not opened

[Stanislav Kinsbursky]

05.03.2012 19:41, Cyrill Gorcunov пишет:
> On Mon, Mar 05, 2012 at 07:29:38PM +0400, Pavel Emelyanov wrote:
>>>
>>>   static inline int fork_with_pid(int pid, unsigned long ns_clone_flags)
>>>   {
>>> +	struct cr_clone_arg ca = { };
>>
>> What for?
>>
>
> This is a good practice to zeroify structures on stack id they
> are not inited via (struct){...} somewhere else or passed into
> function which initizlize it. I think I've overdone here, I'll
> drop this snippet and update the patch.
>
>>>   	int ret = -1;
>>>   	char buf[32];
>>> -	struct cr_clone_arg ca;
>>>   	void *stack;
>>>
>>>   	pr_info("Forking task with %d pid (flags %lx)\n", pid, ns_clone_flags);
>>> @@ -1246,10 +1246,9 @@ static inline int fork_with_pid(int pid, unsigned long ns_clone_flags)
>>>   		goto err;
>>>   	}
>>>
>>> -	snprintf(buf, sizeof(buf), "%d", pid - 1);
>>> -	ca.pid = pid;
>>> -	ca.clone_flags = ns_clone_flags;
>>> -	ca.fd = open(LAST_PID_PATH, O_RDWR);
>>> +	ca.pid		= pid;
>>> +	ca.clone_flags	= ns_clone_flags;
>>> +	ca.fd		= open(LAST_PID_PATH, O_RDWR);
>>
>> In general I don't appreciate such style of assignments.
>
> Sure I'll not toss this lines in new version, but in general
> I think this style of assignment a way more convenient to
> read and parse by eyes. So if you don't mind I'll continue
> using it (if there is strong disagreement -- then sure I'll
> stop using this style).
>

 From my POW the problem here is not in the style you are trying to invent, but 
in the attention you are paying to such things.
I believe that these style efforts are minor - especially comparing to other 
pending tasks in "TODO" list.
IOW, energy, spent on code cleaning, have to be balanced with the energy, spend 
on the project development. The project is still to raw to clean it all the time 
just for cleanness itself.
This is just my opinion. And I'm sorry, if it's offensive.

-- 
Best regards,
Stanislav Kinsbursky