[CRIU] [PATCH 2/5] service: set one exit point

[Ruslan Kuprieiev]

On 09/20/2013 11:45 PM, Andrew Vagin wrote:
> On Fri, Sep 20, 2013 at 11:39:35PM +0400, Ruslan Kuprieiev wrote:
>> On 09/20/2013 11:28 PM, Ruslan Kuprieiev wrote:
>>> On 09/20/2013 11:16 PM, Andrew Vagin wrote:
>>>> On Fri, Sep 20, 2013 at 11:12:07PM +0400, Ruslan Kuprieiev wrote:
>>>>> On 09/19/2013 02:37 PM, Andrey Vagin wrote:
>>>>>> Cc: Ruslan Kuprieiev <kupruser at gmail.com>
>>>>>> Signed-off-by: Andrey Vagin <avagin at openvz.org>
>>>>>> ---
>>>>>>   cr-service.c | 19 ++++++++++++-------
>>>>>>   1 file changed, 12 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/cr-service.c b/cr-service.c
>>>>>> index 1d6f5a6..c385549 100644
>>>>>> --- a/cr-service.c
>>>>>> +++ b/cr-service.c
>>>>>> @@ -202,7 +202,7 @@ err:
>>>>>>   int cr_service(bool daemon_mode)
>>>>>>   {
>>>>>> -    int server_fd;
>>>>>> +    int server_fd = -1;
>>>>>>       int child_pid;
>>>>>>       struct sockaddr_un server_addr;
>>>>>> @@ -218,7 +218,7 @@ int cr_service(bool daemon_mode)
>>>>>>       server_fd = socket(AF_LOCAL, SOCK_SEQPACKET, 0);
>>>>>>       if (server_fd == -1) {
>>>>>>           pr_perror("Can't initialize service socket.");
>>>>>> -        return -1;
>>>>>> +        goto err;
>>>>>>       }
>>>>>>       memset(&server_addr, 0, sizeof(server_addr));
>>>>>> @@ -239,7 +239,7 @@ int cr_service(bool daemon_mode)
>>>>>>       if (bind(server_fd, (struct sockaddr *) &server_addr,
>>>>>>                       server_addr_len) == -1) {
>>>>>>           pr_perror("Can't bind.");
>>>>>> -        return -1;
>>>>>> +        goto err;
>>>>>>       }
>>>>>>       pr_info("The service socket is bound to %s\n",
>>>>>> server_addr.sun_path);
>>>>>> @@ -247,18 +247,18 @@ int cr_service(bool daemon_mode)
>>>>>>       /* change service socket permissions, so anyone can
>>>>>> connect to it */
>>>>>>       if (chmod(server_addr.sun_path, 0666)) {
>>>>>>           pr_perror("Can't change permissions of the service
>>>>>> socket.");
>>>>>> -        return -1;
>>>>>> +        goto err;
>>>>>>       }
>>>>>>       if (listen(server_fd, 16) == -1) {
>>>>>>           pr_perror("Can't listen for socket connections.");
>>>>>> -        return -1;
>>>>>> +        goto err;
>>>>>>       }
>>>>>>       if (daemon_mode) {
>>>>>>           if (daemon(0, 0) == -1) {
>>>>>>               pr_perror("Can't run service server in the background");
>>>>>> -            return -errno;
>>>>>> +            goto err;
>>>>>>           }
>>>>>>       }
>>>>>> @@ -290,5 +290,10 @@ int cr_service(bool daemon_mode)
>>>>>>           }
>>>>>>       }
>>>>>> -    return 0;
>>>>>> +err:
>>>>>> +    close_safe(&server_fd);
>>>>>> +
>>>>>> +    exit(1);
>>>>>> +
>>>>>> +    return -1;
>>>>> Why do we need both exit and return?
>>>>> Will not exit() terminate the process?
>>>> exit() terminates the process, but this function must return anything.
>>> I didn't get that...Who will get returned value, if exit()
>>> terminates the whole process?
>>>
>> I mean, aren't any commands after exit() would not be executed?
> exit() dosn't return, so this "return -1" is redundant
>
> Actually we can delete this "return -1" and gcc will not report any error,
> because exit() is marked as "noreturn".
>
> extern void exit (int __status) __THROW __attribute__ ((__noreturn__));
>
> so I would prefer to delete return -1 from here

Cool! :)
Thank you for detailed answer.

>> Sorry for stupid questions.=)
>>
>>>> We can remove this exit().
>>>>
>>>> Can you check that this series breaks nothing?
>>> Sure=).
>>> Would it be ok, if I will include this series in another patch and
>>> add "Original-patch-by:" ?
>>>> Thanks
>>>>
>>>>>>   }