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

[Andrew Vagin]

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

> 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
> >>
> >>>>  }
> >
>