Require each service to have a unique name. Log service errors using service_error().

--HG--
branch : HEAD

diff --git a/src/master/master-settings.c b/src/master/master-settings.c
index b97b96cd3c998811ef5b583a201be99989e31e9f..46a8bf272872973d78fcfcdb6b84f6202539bd1e 100644 (file)
--- a/src/master/master-settings.c
+++ b/src/master/master-settings.c
@@ -82,6 +82,7 @@ static struct setting_parser_info inet_listener_setting_parser_info = {
 
 static struct setting_define service_setting_defines[] = {
        DEF(SET_INTERNAL, master_set),
+       DEF(SET_STR, name),
        DEF(SET_STR, type),
        DEF(SET_STR, executable),
        DEF(SET_STR, user),
@@ -110,6 +111,7 @@ static struct setting_define service_setting_defines[] = {
 static struct service_settings service_default_settings = {
        MEMBER(master_set) NULL,
 
+       MEMBER(name) "",
        MEMBER(type) "",
        MEMBER(executable) "",
        MEMBER(user) "",
@@ -138,7 +140,7 @@ struct setting_parser_info service_setting_parser_info = {
        MEMBER(dynamic_parsers) NULL,
 
        MEMBER(parent_offset) offsetof(struct service_settings, master_set),
-       MEMBER(type_offset) (size_t)-1,
+       MEMBER(type_offset) offsetof(struct service_settings, name),
        MEMBER(struct_size) sizeof(struct service_settings)
 };
 
@@ -217,7 +219,7 @@ master_settings_verify(void *_set, pool_t pool, const char **error_r)
 {
        const struct master_settings *set = _set;
        struct service_settings *const *services;
-       unsigned int i, count;
+       unsigned int i, j, count;
 
        if (set->last_valid_uid != 0 &&
            set->first_valid_uid > set->last_valid_uid) {
@@ -237,6 +239,21 @@ master_settings_verify(void *_set, pool_t pool, const char **error_r)
                *error_r = "No services defined";
                return FALSE;
        }
+       for (i = 0; i < count; i++) {
+               if (*services[i]->name == '\0') {
+                       *error_r = t_strdup_printf(
+                               "Service #%d is missing name", i);
+                       return FALSE;
+               }
+               for (j = 0; j < i; j++) {
+                       if (strcmp(services[i]->name, services[j]->name) == 0) {
+                               *error_r = t_strdup_printf(
+                                       "Duplicate service name: %s",
+                                       services[i]->name);
+                               return FALSE;
+                       }
+               }
+       }
        for (i = 0; i < count; i++) {
                if (*services[i]->executable != '/') {
                        services[i]->executable =
@@ -253,7 +270,7 @@ master_settings_verify(void *_set, pool_t pool, const char **error_r)
                    *services[i]->chroot != '\0') {
                        *error_r = t_strdup_printf("service(%s): "
                                "drop_priv_before_exec=yes can't be "
-                               "used with chroot", services[i]->executable);
+                               "used with chroot", services[i]->name);
                        return FALSE;
                }
                fix_file_listener_paths(&services[i]->unix_listeners,

diff --git a/src/master/master-settings.h b/src/master/master-settings.h
index 7a1133bcec14552dbd45ec40b149cfbb511f1b79..ead97d6d561816b84a9f58d3e6e64221b68fbb71 100644 (file)
--- a/src/master/master-settings.h
+++ b/src/master/master-settings.h
@@ -17,6 +17,7 @@ struct inet_listener_settings {
 struct service_settings {
        struct master_settings *master_set;
 
+       const char *name;
        const char *type;
        const char *executable;
        const char *user;

diff --git a/src/master/service-auth-server.c b/src/master/service-auth-server.c
index 8c3541827a5bfb3b01dd3c6bc8a6173027b741a2..0996bc9a8b0fa57af52be73d7b26ea3c48be5e03 100644 (file)
--- a/src/master/service-auth-server.c
+++ b/src/master/service-auth-server.c
@@ -66,10 +66,10 @@ auth_process_lookup_request(struct service_process_auth_server *process,
 
        request = hash_table_lookup(process->auth_requests, POINTER_CAST(id));
        if (request == NULL) {
-               i_error("service(%s): authentication service %s "
-                       "sent reply with unknown ID %u",
-                       process->process.service->name,
-                       dec2str(process->process.pid), id);
+               service_error(process->process.service,
+                             "authentication service %s "
+                             "sent reply with unknown ID %u",
+                             dec2str(process->process.pid), id);
                return NULL;
        }
 
@@ -181,9 +181,10 @@ service_process_auth_server_input(struct service_process_auth_server *process)
                return;
        case -2:
                /* buffer full */
-               i_error("service(%s): authentication server process %s "
-                       "sent us too long line", process->process.service->name,
-                       dec2str(process->process.pid));
+               service_error(process->process.service,
+                             "authentication server process %s "
+                             "sent us too long line",
+                             dec2str(process->process.pid));
                service_process_auth_server_close(process);
                return;
        }
@@ -197,11 +198,11 @@ service_process_auth_server_input(struct service_process_auth_server *process)
                if (strncmp(line, "VERSION\t", 8) != 0 ||
                    atoi(t_strcut(line + 8, '\t')) !=
                    AUTH_MASTER_PROTOCOL_MAJOR_VERSION) {
-                       i_error("service(%s): authentication server process %s "
-                               "not compatible with master process "
-                               "(mixed old and new binaries?)",
-                               process->process.service->name,
-                               dec2str(process->process.pid));
+                       service_error(process->process.service,
+                                     "authentication server process %s "
+                                     "not compatible with master process "
+                                     "(mixed old and new binaries?)",
+                                     dec2str(process->process.pid));
                        service_process_auth_server_close(process);
                        return;
                }

diff --git a/src/master/service-auth-source.c b/src/master/service-auth-source.c
index 057056f437e804ea88eee2fc388d1b8d9d0a4b73..28798f89ba52d3870301b4696d83a295b25bf5ee 100644 (file)
--- a/src/master/service-auth-source.c
+++ b/src/master/service-auth-source.c
@@ -128,22 +128,20 @@ auth_read_request(struct service_process_auth_source *process,
                        /* disconnected */
                } else if (ret > 0) {
                        /* request wasn't fully read */
-                       i_error("service(%s): fd_read() partial input (%d/%d)",
-                               service->name, (int)ret, (int)sizeof(*req));
+                       service_error(service, "fd_read() partial input (%d/%d)",
+                                     (int)ret, (int)sizeof(*req));
                } else {
                        if (errno == EAGAIN)
                                return 0;
 
-                       i_error("service(%s): fd_read() failed: %m",
-                               service->name);
+                       service_error(service, "fd_read() failed: %m");
                }
                return -1;
        }
 
        if (req->data_size != 0) {
                if (req->data_size > MASTER_AUTH_MAX_DATA_SIZE) {
-                       i_error("service(%s): Too large auth data_size sent",
-                               service->name);
+                       service_error(service, "Too large auth data_size sent");
                        return -1;
                }
                /* @UNSAFE */
@@ -153,30 +151,28 @@ auth_read_request(struct service_process_auth_source *process,
                                /* disconnected */
                        } else if (ret > 0) {
                                /* request wasn't fully read */
-                               i_error("service(%s): Data read partially %d/%u",
-                                       service->name, (int)ret, req->data_size);
+                               service_error(service,
+                                             "Data read partially %d/%u",
+                                             (int)ret, req->data_size);
                        } else {
-                               i_error("service(%s): read(data) failed: %m",
-                                       service->name);
+                               service_error(service, "read(data) failed: %m");
                        }
                        return -1;
                }
        }
 
        if (*client_fd_r == -1) {
-               i_error("service(%s): Auth request missing a file descriptor",
-                       service->name);
+               service_error(service, "Auth request missing a file descriptor");
                return -1;
        }
 
        if (fstat(*client_fd_r, &st) < 0) {
-               i_error("service(%s): fstat(auth dest fd) failed: %m",
-                       service->name);
+               service_error(service, "fstat(auth dest fd) failed: %m");
                return -1;
        }
        if (st.st_ino != req->ino) {
-               i_error("service(%s): Auth request inode mismatch: %s != %s",
-                       service->name, dec2str(st.st_ino), dec2str(req->ino));
+               service_error(service, "Auth request inode mismatch: %s != %s",
+                             dec2str(st.st_ino), dec2str(req->ino));
                return -1;
        }
        return 1;
@@ -210,9 +206,8 @@ service_process_auth_source_input(struct service_process_auth_source *process)
        /* we have a request. check its validity. */
        auth_process = hash_table_lookup(service->list->pids, &req.auth_pid);
        if (auth_process == NULL) {
-               i_error("service(%s): authentication request for unknown "
-                       "auth server PID %s", service->name,
-                       dec2str(req.auth_pid));
+               service_error(service, "authentication request for unknown "
+                             "auth server PID %s", dec2str(req.auth_pid));
                service_process_auth_source_send_reply(process, req.tag, FALSE);
                (void)close(client_fd);
                return;
@@ -224,7 +219,7 @@ service_process_auth_source_input(struct service_process_auth_source *process)
                    ioloop_time - AUTH_BUSY_LOG_INTERVAL) {
                        i_warning("service(%s): authentication server PID "
                                  "%s too busy",
-                                 auth_process->process.service->name,
+                                 auth_process->process.service->set->name,
                                  dec2str(req.auth_pid));
                         auth_process->auth_busy_stamp = ioloop_time;
                }

diff --git a/src/master/service-listen.c b/src/master/service-listen.c
index 2a077100841392261c7eacc223d4675404d55589..4345134f154a27a1444897f793c7f33377d216c0 100644 (file)
--- a/src/master/service-listen.c
+++ b/src/master/service-listen.c
@@ -31,8 +31,8 @@ static int service_unix_listener_listen(struct service_listener *l)
                }
 
                if (errno != EADDRINUSE) {
-                       i_error("service(%s): net_listen_unix(%s) failed: %m",
-                               service->name, set->path);
+                       service_error(service, "net_listen_unix(%s) failed: %m",
+                                     set->path);
                        return -1;
                }
 
@@ -42,15 +42,15 @@ static int service_unix_listener_listen(struct service_listener *l)
                if (fd != -1 || errno != ECONNREFUSED || i >= 3) {
                        if (fd != -1)
                                (void)close(fd);
-                       i_error("service(%s): Socket already exists: %s",
-                               service->name, set->path);
+                       service_error(service, "Socket already exists: %s",
+                                     set->path);
                        return 0;
                }
 
                /* delete and try again */
                if (unlink(set->path) < 0 && errno != ENOENT) {
-                       i_error("service(%s): unlink(%s) failed: %m",
-                               service->name, set->path);
+                       service_error(service, "unlink(%s) failed: %m",
+                                     set->path);
                        return -1;
                }
        }
@@ -88,15 +88,13 @@ static int service_fifo_listener_listen(struct service_listener *l)
        umask(old_umask);
 
        if (ret < 0 && errno != EEXIST) {
-               i_error("service(%s): mkfifo(%s) failed: %m",
-                       service->name, set->path);
+               service_error(service, "mkfifo(%s) failed: %m", set->path);
                return -1;
        }
 
        fd = open(set->path, O_RDONLY);
        if (fd == -1) {
-               i_error("service(%s): open(%s) failed: %m",
-                       service->name, set->path);
+               service_error(service, "open(%s) failed: %m", set->path);
                return -1;
        }
 
@@ -127,8 +125,8 @@ static int service_inet_listener_listen(struct service_listener *l)
 
        fd = net_listen(&l->set.inetset.ip, &port, service->process_limit);
        if (fd < 0) {
-               i_error("service(%s): listen(%s, %u) failed: %m",
-                       service->name, set->address, set->port);
+               service_error(service, "listen(%s, %u) failed: %m",
+                             set->address, set->port);
                return errno == EADDRINUSE ? 0 : -1;
        }
        net_set_nonblock(fd, TRUE);

diff --git a/src/master/service-log.c b/src/master/service-log.c
index 7566a52b63f7d491e8851abda46b5324dde0b087..c1dc3fbfe42efeaa4e7af2ecd3ecb3a51c72e78e 100644 (file)
--- a/src/master/service-log.c
+++ b/src/master/service-log.c
@@ -37,12 +37,12 @@ int services_log_init(struct service_list *service_list)
                fd_close_on_exec(services[i]->log_fd[0], TRUE);
                fd_close_on_exec(services[i]->log_fd[1], TRUE);
 
-               handshake.prefix_len = strlen(services[i]->name) + 2;
+               handshake.prefix_len = strlen(services[i]->set->name) + 2;
 
                buffer_set_used_size(handshake_buf, 0);
                buffer_append(handshake_buf, &handshake, sizeof(handshake));
-               buffer_append(handshake_buf, services[i]->name,
-                             strlen(services[i]->name));
+               buffer_append(handshake_buf, services[i]->set->name,
+                             strlen(services[i]->set->name));
                buffer_append(handshake_buf, ": ", 2);
 
                ret = write(services[i]->log_fd[1],
@@ -89,12 +89,12 @@ void services_log_deinit(struct service_list *service_list)
        for (i = 0; i < count; i++) {
                if (services[i]->log_fd[0] != -1) {
                        if (close(services[i]->log_fd[0]) < 0) {
-                               i_error("service(%s): close(log_fd) failed: %m",
-                                       services[i]->name);
+                               service_error(services[i],
+                                             "close(log_fd) failed: %m");
                        }
                        if (close(services[i]->log_fd[1]) < 0) {
-                               i_error("service(%s): close(log_fd) failed: %m",
-                                       services[i]->name);
+                               service_error(services[i],
+                                             "close(log_fd) failed: %m");
                        }
                        services[i]->log_fd[0] = -1;
                        services[i]->log_fd[1] = -1;

diff --git a/src/master/service-monitor.c b/src/master/service-monitor.c
index 0ede21dd892161a0bdf64f0dc4ada63b6e009425..d92db983bb7e2f600349c1943452006e517f8785 100644 (file)
--- a/src/master/service-monitor.c
+++ b/src/master/service-monitor.c
@@ -29,17 +29,16 @@ static void service_status_input(struct service *service)
        ret = read(service->status_fd[0], &status, sizeof(status));
        switch (ret) {
        case 0:
-               i_error("service(%s): read(status) failed: EOF", service->name);
+               service_error(service, "read(status) failed: EOF");
                service_monitor_stop(service);
                return;
        case -1:
-               i_error("service(%s): read(status) failed: %m", service->name);
+               service_error(service, "read(status) failed: %m");
                service_monitor_stop(service);
                return;
        default:
-               i_error("service(%s): child %s sent partial status update "
-                       "(%d bytes)", service->name,
-                       dec2str(status.pid), (int)ret);
+               service_error(service, "child %s sent partial status update "
+                             "(%d bytes)", dec2str(status.pid), (int)ret);
                return;
 
        case sizeof(status):
@@ -62,9 +61,8 @@ static void service_status_input(struct service *service)
                   randomness here, but the worst they can do is DoS and there
                   are already more serious problems if someone is able to do
                   this.. */
-               i_error("service(%s): Ignoring invalid update from child %s "
-                       "(UID=%u)", service->name, dec2str(status.pid),
-                       status.uid);
+               service_error(service, "Ignoring invalid update from child %s "
+                             "(UID=%u)", dec2str(status.pid), status.uid);
                return;
        }
 
@@ -111,8 +109,7 @@ static void service_monitor_throttle(struct service *service)
        if (service->to_throttle != NULL)
                return;
 
-       i_error("service(%s): command startup failed, throttling",
-               service->name);
+       service_error(service, "command startup failed, throttling");
        service_monitor_listen_stop(service);
 
        service->to_throttle = timeout_add(THROTTLE_TIMEOUT,
@@ -180,8 +177,7 @@ void services_monitor_start(struct service_list *service_list)
                if (services[i]->status_fd[0] == -1) {
                        /* we haven't yet created status pipe */
                        if (pipe(services[i]->status_fd) < 0) {
-                               i_error("service(%s): pipe() failed: %m",
-                                       services[i]->name);
+                               service_error(services[i], "pipe() failed: %m");
                                continue;
                        }
 
@@ -215,10 +211,10 @@ static void service_monitor_stop(struct service *service)
        if (service->status_fd[0] != -1) {
                for (i = 0; i < 2; i++) {
                        if (close(service->status_fd[i]) < 0) {
-                               i_error("service(%s): close(%d) failed: %m",
-                                       service->name, i);
+                               service_error(service,
+                                             "close(status fd) failed: %m");
                        }
-                        service->status_fd[i] = -1;
+                       service->status_fd[i] = -1;
                }
        }
        service_monitor_listen_stop(service);

diff --git a/src/master/service-process.c b/src/master/service-process.c
index 1286b9fe90a215b6c5ec87cf8f4fb37c12c97223..eb78edbba245750c085808c8ec4e39946b01d7b2 100644 (file)
--- a/src/master/service-process.c
+++ b/src/master/service-process.c
@@ -99,7 +99,7 @@ service_dup_fds(struct service *service, int auth_fd, int std_fd)
        closelog();
 
        if (dup2_array(&dups) < 0)
-               i_fatal("service(%s): dup2s failed", service->name);
+               service_error(service, "dup2s failed");
 
 #ifdef DEBUG
        env_put(t_strdup_printf("SOCKET_COUNT=%d", socket_listener_count));
@@ -290,12 +290,13 @@ service_process_setup_environment(struct service *service, unsigned int uid)
 
 static void service_process_status_timeout(struct service_process *process)
 {
-       i_error("service(%s): Initial status notification not received in %d "
-               "seconds, killing the process", process->service->name,
-               SERVICE_FIRST_STATUS_TIMEOUT_SECS);
+       service_error(process->service,
+                     "Initial status notification not received in %d "
+                     "seconds, killing the process",
+                     SERVICE_FIRST_STATUS_TIMEOUT_SECS);
        if (kill(process->pid, SIGKILL) < 0 && errno != ESRCH) {
-               i_error("service(%s): kill(%s, SIGKILL) failed: %m",
-                       process->service->name, dec2str(process->pid));
+               service_error(process->service, "kill(%s, SIGKILL) failed: %m",
+                             dec2str(process->pid));
        }
        timeout_remove(&process->to_status);
 }
@@ -315,8 +316,7 @@ service_process_create(struct service *service, const char *const *auth_args,
        case SERVICE_TYPE_AUTH_SOURCE:
        case SERVICE_TYPE_AUTH_SERVER:
                if (socketpair(AF_UNIX, SOCK_STREAM, 0, fd) < 0) {
-                       i_error("service(%s): socketpair() failed: %m",
-                               service->name);
+                       service_error(service, "socketpair() failed: %m");
                        return NULL;
                }
                fd_close_on_exec(fd[0], TRUE);
@@ -329,7 +329,7 @@ service_process_create(struct service *service, const char *const *auth_args,
 
        pid = fork();
        if (pid < 0) {
-               i_error("service(%s): fork() failed: %m", service->name);
+               service_error(service, "fork() failed: %m");
                if (fd[0] != -1) {
                        (void)close(fd[0]);
                        (void)close(fd[1]);
@@ -543,7 +543,7 @@ service_process_get_status_error(string_t *str, struct service_process *process,
 
        *type_r = LOG_TYPE_ERROR;
 
-       str_printfa(str, "service(%s): child %s ", service->name,
+       str_printfa(str, "service(%s): child %s ", service->set->name,
                    dec2str(process->pid));
        if (WIFSIGNALED(status)) {
                str_printfa(str, "killed with signal %d", WTERMSIG(status));

diff --git a/src/master/service.c b/src/master/service.c
index f6b0656158c52d9bff1b99fb758a306470371c03..b46f9792cf6d943f1930c23368e9583b81083254 100644 (file)
--- a/src/master/service.c
+++ b/src/master/service.c
@@ -13,6 +13,16 @@
 #include <pwd.h>
 #include <grp.h>
 
+void service_error(struct service *service, const char *format, ...)
+{
+       va_list args;
+
+       va_start(args, format);
+       i_error("service(%s): %s", service->set->name,
+               t_strdup_vprintf(format, args));
+       va_end(args);
+}
+
 static int get_uid(const char *user, uid_t *uid_r, const char **error_r)
 {
        struct passwd *pw;
@@ -148,7 +158,7 @@ service_create(pool_t pool, const struct service_settings *set,
        struct inet_listener_settings *const *inet_listeners;
        struct service *service;
         struct service_listener *l;
-       const char *p, *const *tmp;
+       const char *const *tmp;
        string_t *str;
        unsigned int i, unix_count, fifo_count, inet_count;
 
@@ -158,15 +168,12 @@ service_create(pool_t pool, const struct service_settings *set,
 
        service->type = SERVICE_TYPE_UNKNOWN;
        if (*set->type != '\0') {
-               service->name = set->type;
                if (strcmp(set->type, "log") == 0)
                        service->type = SERVICE_TYPE_LOG;
                else if (strcmp(set->type, "config") == 0)
                        service->type = SERVICE_TYPE_CONFIG;
                else if (strcmp(set->type, "auth") == 0)
                        service->type = SERVICE_TYPE_AUTH_SERVER;
-               else
-                       service->name = NULL;
        }
 
        if (*set->auth_dest_service != '\0')
@@ -188,17 +195,6 @@ service_create(pool_t pool, const struct service_settings *set,
                return NULL;
        }
 
-       /* get service name from some of the unique types, fallback to
-          executable name without path and parameters */
-       if (service->name == NULL) {
-               p = strrchr(set->executable, '/');
-               if (p == NULL)
-                       service->name = t_strcut(set->executable, ' ');
-               else
-                       service->name = t_strcut(p + 1, ' ');
-               service->name = p_strdup(pool, service->name);
-       }
-
        if (get_uid(set->user, &service->uid, error_r) < 0)
                return NULL;
        if (get_gid(set->group, &service->gid, error_r) < 0)
@@ -318,7 +314,7 @@ service_lookup(struct service_list *service_list, const char *name)
 
        services = array_get(&service_list->services, &count);
        for (i = 0; i < count; i++) {
-               if (strcmp(services[i]->name, name) == 0)
+               if (strcmp(services[i]->set->name, name) == 0)
                        return services[i];
        }
        return NULL;
@@ -349,8 +345,7 @@ services_create(const struct master_settings *set,
                                         service_list, &error);
                if (service == NULL) {
                        *error_r = t_strdup_printf("service(%s) %s",
-                               service_settings[i]->type == NULL ? "unknown" :
-                               service_settings[i]->type, error);
+                               service_settings[i]->name, error);
                        return NULL;
                }
 
@@ -421,8 +416,8 @@ void service_signal(struct service *service, int signo)
                        continue;
 
                if (kill(process->pid, signo) < 0 && errno != ESRCH) {
-                       i_error("service(%s): kill(%s, %d) failed: %m",
-                               service->name, dec2str(process->pid), signo);
+                       service_error(service, "kill(%s, %d) failed: %m",
+                                     dec2str(process->pid), signo);
                }
        }
        hash_table_iterate_deinit(&iter);

diff --git a/src/master/service.h b/src/master/service.h
index 1dd71b95cdf7ff34e36eb08b583983dd230942b4..bf4bd5794feb5a13348feccd22c848522d7f3896 100644 (file)
--- a/src/master/service.h
+++ b/src/master/service.h
@@ -45,7 +45,6 @@ struct service {
        struct service_list *list;
 
        enum service_type type;
-       const char *name;
 
        const struct service_settings *set;
        const char *config_file_path;
@@ -107,4 +106,7 @@ void services_destroy(struct service_list *service_list);
 /* Send a signal to all processes in a given service */
 void service_signal(struct service *service, int signo);
 
+void service_error(struct service *service, const char *format, ...)
+       ATTR_FORMAT(2, 3);
+
 #endif