]> git.ipfire.org Git - thirdparty/FORT-validator.git/commitdiff
Code review
authorAlberto Leiva Popper <ydahhrk@gmail.com>
Wed, 26 May 2021 01:17:09 +0000 (20:17 -0500)
committerAlberto Leiva Popper <ydahhrk@gmail.com>
Wed, 26 May 2021 01:17:09 +0000 (20:17 -0500)
- Main validation loop: Remove some confusing, seemingly needless
  wrapper functions.
- Libcurl: Catch lots of status codes properly
- Libcurl: Send proper data types to curl_easy_setopt()
  (Argument types were not matching documented requirements.)
- RTR server: Reduce argument lists.

13 files changed:
src/http/http.c
src/main.c
src/object/certificate.c
src/object/tal.c
src/rrdp/db/db_rrdp.c
src/rsync/rsync.c
src/rsync/rsync.h
src/rtr/db/vrps.c
src/rtr/db/vrps.h
src/rtr/rtr.c
src/rtr/rtr.h
src/uri.c
src/validation_run.c

index 7ecfab247d2125c47bb81fc7eed569277cf4f391..dd497d80cbad1272a27bbeb313f9611604b63c94 100644 (file)
@@ -42,44 +42,79 @@ http_cleanup(void)
        curl_global_cleanup();
 }
 
+static void
+setopt_str(CURL *curl, CURLoption opt, char const *value)
+{
+       CURLcode result;
+
+       if (value == NULL)
+               return;
+
+       result = curl_easy_setopt(curl, opt, value);
+       if (result != CURLE_OK) {
+               fprintf(stderr, "curl_easy_setopt(%d, %s) returned %d: %s\n",
+                   opt, value, result, curl_easy_strerror(result));
+       }
+}
+
+static void
+setopt_long(CURL *curl, CURLoption opt, long value)
+{
+       CURLcode result;
+
+       result = curl_easy_setopt(curl, opt, value);
+       if (result != CURLE_OK) {
+               fprintf(stderr, "curl_easy_setopt(%d, %ld) returned %d: %s\n",
+                   opt, value, result, curl_easy_strerror(result));
+       }
+}
+
+static void
+setopt_writefunction(CURL *curl, http_write_cb cb, void *arg)
+{
+       CURLcode result;
+
+       result = curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, cb);
+       if (result != CURLE_OK) {
+               fprintf(stderr, "curl_easy_setopt(%d) returned %d: %s\n",
+                   CURLOPT_WRITEFUNCTION, result, curl_easy_strerror(result));
+       }
+
+       result = curl_easy_setopt(curl, CURLOPT_WRITEDATA, arg);
+       if (result != CURLE_OK) {
+               fprintf(stderr, "curl_easy_setopt(%d) returned %d: %s\n",
+                   CURLOPT_WRITEDATA, result, curl_easy_strerror(result));
+       }
+}
+
 static int
 http_easy_init(struct http_handler *handler)
 {
-       CURL *tmp;
+       CURL *result;
+       long timeout;
 
-       tmp = curl_easy_init();
-       if (tmp == NULL)
+       result = curl_easy_init();
+       if (result == NULL)
                return pr_enomem();
 
-       /* Use header always */
-       if (config_get_http_user_agent() != NULL)
-               curl_easy_setopt(tmp, CURLOPT_USERAGENT,
-                   config_get_http_user_agent());
-       /* Only utilizes if indicated, otherwise use system default */
-       if (config_get_http_ca_path() != NULL)
-               curl_easy_setopt(tmp, CURLOPT_CAPATH,
-                   config_get_http_ca_path());
+       setopt_str(result, CURLOPT_USERAGENT, config_get_http_user_agent());
 
-       curl_easy_setopt(tmp, CURLOPT_CONNECTTIMEOUT,
+       setopt_long(result, CURLOPT_CONNECTTIMEOUT,
            config_get_http_connect_timeout());
-       curl_easy_setopt(tmp, CURLOPT_TIMEOUT,
+       setopt_long(result, CURLOPT_TIMEOUT,
            config_get_http_transfer_timeout());
-       if (config_get_http_idle_timeout() > 0) {
-               curl_easy_setopt(tmp, CURLOPT_LOW_SPEED_TIME,
-                   config_get_http_idle_timeout());
-               curl_easy_setopt(tmp, CURLOPT_LOW_SPEED_LIMIT, 1);
-       } else {
-               /* Disabled */
-               curl_easy_setopt(tmp, CURLOPT_LOW_SPEED_TIME, 0);
-               curl_easy_setopt(tmp, CURLOPT_LOW_SPEED_LIMIT, 0);
-       }
+
+       timeout = config_get_http_idle_timeout();
+       setopt_long(result, CURLOPT_LOW_SPEED_TIME, timeout);
+       setopt_long(result, CURLOPT_LOW_SPEED_LIMIT, !!timeout);
 
        /* Always expect HTTPS usage */
-       curl_easy_setopt(tmp, CURLOPT_SSL_VERIFYHOST, 2);
-       curl_easy_setopt(tmp, CURLOPT_SSL_VERIFYPEER, 1);
+       setopt_long(result, CURLOPT_SSL_VERIFYHOST, 2L);
+       setopt_long(result, CURLOPT_SSL_VERIFYPEER, 1L);
+       setopt_str(result, CURLOPT_CAPATH, config_get_http_ca_path());
 
        /* Currently all requests use GET */
-       curl_easy_setopt(tmp, CURLOPT_HTTPGET, 1);
+       setopt_long(result, CURLOPT_HTTPGET, 1L);
 
        /*
         * Response codes >= 400 will be treated as errors
@@ -90,16 +125,15 @@ http_easy_init(struct http_handler *handler)
         *
         * Well, be ready for those scenarios when performing the requests.
         */
-       curl_easy_setopt(tmp, CURLOPT_FAILONERROR, 1L);
+       setopt_long(result, CURLOPT_FAILONERROR, 1L);
 
        /* Refer to its error buffer */
-       curl_easy_setopt(tmp, CURLOPT_ERRORBUFFER, handler->errbuf);
+       setopt_str(result, CURLOPT_ERRORBUFFER, handler->errbuf);
 
        /* Prepare for multithreading, avoid signals */
-       curl_easy_setopt(tmp, CURLOPT_NOSIGNAL, 1L);
-
-       handler->curl = tmp;
+       setopt_long(result, CURLOPT_NOSIGNAL, 1L);
 
+       handler->curl = result;
        return 0;
 }
 
@@ -117,28 +151,61 @@ static int
 http_fetch(struct http_handler *handler, char const *uri, long *response_code,
     long *cond_met, bool log_operation, http_write_cb cb, void *arg)
 {
-       CURLcode res;
+       CURLcode res, res2;
        long unmet = 0;
 
        handler->errbuf[0] = 0;
-       curl_easy_setopt(handler->curl, CURLOPT_URL, uri);
-       curl_easy_setopt(handler->curl, CURLOPT_WRITEFUNCTION, cb);
-       curl_easy_setopt(handler->curl, CURLOPT_WRITEDATA, arg);
+       setopt_str(handler->curl, CURLOPT_URL, uri);
+       setopt_writefunction(handler->curl, cb, arg);
 
        pr_val_debug("Doing HTTP GET to '%s'.", uri);
        res = curl_easy_perform(handler->curl);
-       curl_easy_getinfo(handler->curl, CURLINFO_RESPONSE_CODE, response_code);
+
+       res2 = curl_easy_getinfo(handler->curl, CURLINFO_RESPONSE_CODE,
+           response_code);
+       if (res2 != CURLE_OK) {
+               return pr_op_err("curl_easy_getinfo(CURLINFO_RESPONSE_CODE) returned %d (%s). I think this is supposed to be illegal, so I'll have to drop URI '%s'.",
+                   res2, curl_err_string(handler, res2), uri);
+       }
+
        if (res == CURLE_OK) {
                if (*response_code != HTTP_OK)
                        return 0;
+
                /*
-                * Scenario: Received an OK code, but the time condition wasn't
-                * actually met, handle this as a "Not Modified" code.
-                *
-                * This check is due to old libcurl versions, where the impl
+                * Scenario: Received an OK code, but the time condition
+                * (if-modified-since) wasn't actually met (ie. the document
+                * has not been modified since we last requested it), so handle
+                * this as a "Not Modified" code.
+                *
+                * This check is due to old libcurl versions, where the impl
                 * doesn't let us get the response content since the library
                 * does the time validation, resulting in "The requested
                 * document is not new enough".
+                *
+                * Update 2021-05-25: I just tested libcurl in my old CentOS 7.7
+                * VM (which is from October 2019, ie. older than the comments
+                * above), and it behaves normally.
+                * Also, the changelog doesn't mention anything about
+                * If-Modified-Since.
+                * This glue code is suspicious to me.
+                *
+                * For the record, this is how it behaves in my today's Ubuntu,
+                * as well as my Centos 7.7.1908:
+                *
+                *      if if-modified-since is included:
+                *              if page was modified:
+                *                      HTTP 200
+                *                      unmet: 0
+                *                      writefunction called
+                *              else:
+                *                      HTTP 304
+                *                      unmet: 1
+                *                      writefunction not called
+                *      else:
+                *              HTTP OK
+                *              unmet: 0
+                *              writefunction called
                 */
                res = curl_easy_getinfo(handler->curl, CURLINFO_CONDITION_UNMET,
                    &unmet);
@@ -229,8 +296,8 @@ __http_download_file(struct rpki_uri *uri, long *response_code, long ims_value,
 
                /* Set "If-Modified-Since" header only if a value is specified */
                if (ims_value > 0) {
-                       curl_easy_setopt(handler.curl, CURLOPT_TIMEVALUE, ims_value);
-                       curl_easy_setopt(handler.curl, CURLOPT_TIMECONDITION,
+                       setopt_long(handler.curl, CURLOPT_TIMEVALUE, ims_value);
+                       setopt_long(handler.curl, CURLOPT_TIMECONDITION,
                            CURL_TIMECOND_IFMODSINCE);
                }
                error = http_fetch(&handler, uri_get_global(uri), response_code,
@@ -335,6 +402,17 @@ http_download_file_with_ims(struct rpki_uri *uri, long value,
        if (cond_met)
                return 0;
 
+       /*
+        * Situation:
+        *
+        * - old libcurl (because libcurl returned HTTP 200 and cond_met == 0,
+        *   which is a contradiction)
+        * - the download was successful (error == 0)
+        * - the page WAS modified since the last update
+        *
+        * libcurl wrote an empty file, so we have to redownload.
+        */
+
        return __http_download_file(uri, &response, 0, &cond_met,
            log_operation);
 
index 0b18a20d3146472d309ff9cf24131b92e3d432c1..cefd4c1df1094bd4b5569a7bd3a7426d36361091 100644 (file)
@@ -5,6 +5,7 @@
 #include "nid.h"
 #include "reqs_errors.h"
 #include "thread_var.h"
+#include "validation_run.h"
 #include "http/http.h"
 #include "rtr/rtr.h"
 #include "rtr/db/vrps.h"
 #include "rrdp/db/db_rrdp.h"
 
 static int
-start_rtr_server(void)
+run_rtr_server(void)
 {
        int error;
 
-       error = vrps_init();
-       if (error)
-               goto just_quit;
-
-       error = db_rrdp_init();
-       if (error)
-               goto vrps_cleanup;
-
-       error = reqs_errors_init();
+       error = rtr_start();
        if (error)
-               goto db_rrdp_cleanup;
+               return error;
 
-       error = rtr_listen();
+       error = validation_run_first();
+       if (!error)
+               error = validation_run_cycle(); /* Usually loops forever */
 
-       reqs_errors_cleanup();
-db_rrdp_cleanup:
-       db_rrdp_cleanup();
-vrps_cleanup:
-       vrps_destroy();
-just_quit:
+       rtr_stop();
        return error;
 }
 
-static int
-__main(int argc, char **argv)
+int
+main(int argc, char **argv)
 {
        int error;
 
-       error = thvar_init();
+       /* Initializations */
+
+       error = log_setup();
        if (error)
                return error;
+       error = thvar_init();
+       if (error)
+               goto revert_log;
        error = incidence_init();
        if (error)
-               return error;
-
+               goto revert_log;
        error = handle_flags_config(argc, argv);
        if (error)
-               return error;
-
+               goto revert_log;
        error = nid_init();
        if (error)
                goto revert_config;
        error = extension_init();
        if (error)
                goto revert_nid;
-
        error = http_init();
        if (error)
                goto revert_nid;
 
+       /*
+        * TODO this looks like a lot of overhead. Is it really necessary
+        * when mode is STANDALONE?
+        */
        error = internal_pool_init();
        if (error)
                goto revert_http;
@@ -73,9 +69,39 @@ __main(int argc, char **argv)
        error = relax_ng_init();
        if (error)
                goto revert_pool;
-
-       error = start_rtr_server();
-
+       error = vrps_init();
+       if (error)
+               goto revert_relax_ng;
+       error = db_rrdp_init();
+       if (error)
+               goto vrps_cleanup;
+       error = reqs_errors_init();
+       if (error)
+               goto db_rrdp_cleanup;
+       error = clients_db_init();
+       if (error)
+               goto revert_reqs_errors;
+
+       /* Do stuff */
+       switch (config_get_mode()) {
+       case STANDALONE:
+               error = validation_run_first();
+               break;
+       case SERVER:
+               error = run_rtr_server();
+               break;
+       }
+
+       /* End */
+
+       clients_db_destroy();
+revert_reqs_errors:
+       reqs_errors_cleanup();
+db_rrdp_cleanup:
+       db_rrdp_cleanup();
+vrps_cleanup:
+       vrps_destroy();
+revert_relax_ng:
        relax_ng_cleanup();
 revert_pool:
        internal_pool_cleanup();
@@ -85,21 +111,7 @@ revert_nid:
        nid_destroy();
 revert_config:
        free_rpki_config();
-       return error;
-}
-
-int
-main(int argc, char **argv)
-{
-       int error;
-
-       error = log_setup();
-       if (error)
-               return error;
-
-       error = __main(argc, argv);
-
+revert_log:
        log_teardown();
-
        return error;
 }
index d3f173e32620e1137b14db6c2a9dfdffa56431e0..9f5fb0db0ccfe0be07b52e1948f37e75f4787e28 100644 (file)
@@ -1919,7 +1919,7 @@ force_aia_validation(struct rpki_uri *caIssuers, X509 *son)
 
        /* RSYNC is still the preferred access mechanism, force the sync */
        do {
-               error = download_files(caIssuers, false, true);
+               error = rsync_download_files(caIssuers, false, true);
                if (!error)
                        break;
                if (error == EREQFAILED) {
@@ -2131,7 +2131,7 @@ exec_rsync_method(struct sia_ca_uris *sia_uris)
 
        /* Stop working on the RRDP local workspace */
        db_rrdp_uris_workspace_disable();
-       error = download_files(sia_uris->caRepository.uri, false, false);
+       error = rsync_download_files(sia_uris->caRepository.uri, false, false);
        if (error)
                return error;
 
@@ -2556,7 +2556,7 @@ certificate_traverse(struct rpp *rpp_parent, struct rpki_uri *cert_uri)
                 */
                pr_val_info("Retrying repository download to discard 'transient inconsistency' manifest issue (see RFC 6481 section 5) '%s'",
                    uri_val_get_printable(sia_uris.caRepository.uri));
-               error = download_files(sia_uris.caRepository.uri, false, true);
+               error = rsync_download_files(sia_uris.caRepository.uri, false, true);
                if (error)
                        break;
 
index 164596c7a10fa655f8135eb4dfc9de898b093b08..870cdc12777c94bd0d29fcfec7339d2dff4f98b6 100644 (file)
@@ -491,13 +491,6 @@ tal_get_spki(struct tal *tal, unsigned char const **buffer, size_t *len)
        *len = tal->spki_len;
 }
 
-static int
-handle_https_uri(struct rpki_uri *uri)
-{
-       return http_download_file(uri,
-           reqs_errors_log_uri(uri_get_global(uri)));
-}
-
 /**
  * Performs the whole validation walkthrough on uri @uri, which is assumed to
  * have been extracted from a TAL.
@@ -535,38 +528,37 @@ handle_tal_uri(struct tal *tal, struct rpki_uri *uri, void *arg)
        if (error)
                return ENSURE_NEGATIVE(error);
 
-       do {
-               if (!thread_arg->sync_files) {
-                       /* Look for local files */
-                       if (!valid_file_or_dir(uri_get_local(uri), true, false,
-                           __pr_val_err)) {
-                               validation_destroy(state);
-                               return 0; /* Error already logged */
-                       }
-                       break;
-               }
-               /* Trying to sync, considering that the sync can be disabled */
+       if (thread_arg->sync_files) {
                if (uri_is_rsync(uri)) {
                        if (!config_get_rsync_enabled()) {
                                validation_destroy(state);
                                return 0; /* Soft error */
                        }
-                       error = download_files(uri, true, false);
-                       break;
+                       error = rsync_download_files(uri, true, false);
+               } else /* HTTPS */ {
+                       if (!config_get_http_enabled()) {
+                               validation_destroy(state);
+                               return 0; /* Soft error */
+                       }
+                       error = http_download_file(uri,
+                           reqs_errors_log_uri(uri_get_global(uri)));
+               }
+
+               /* Reminder: there's a positive error: EREQFAILED */
+               if (error) {
+                       working_repo_push(uri_get_global(uri));
+                       validation_destroy(state);
+                       return pr_val_warn(
+                           "TAL URI '%s' could not be downloaded.",
+                           uri_val_get_printable(uri));
                }
-               if (!config_get_http_enabled()) {
+       } else {
+               /* Look for local files */
+               if (!valid_file_or_dir(uri_get_local(uri), true, false,
+                   __pr_val_err)) {
                        validation_destroy(state);
-                       return 0; /* Soft error */
+                       return 0; /* Error already logged */
                }
-               error = handle_https_uri(uri);
-       } while (0);
-
-       /* Friendly reminder: there's a positive error - EREQFAILED */
-       if (error) {
-               working_repo_push(uri_get_global(uri));
-               validation_destroy(state);
-               return pr_val_warn("TAL URI '%s' could not be downloaded.",
-                   uri_val_get_printable(uri));
        }
 
        /* At least one URI was sync'd */
index 45fd480a75c158e4b0c0f47ee28140359d726d47..7bf328a6abd7ad0ab3bb76e5170c55fe61f89fe6 100644 (file)
@@ -250,7 +250,6 @@ db_rrdp_reset_visited_tals(void)
        rwlock_write_lock(&lock);
        SLIST_FOREACH(found, &db.tals, next)
                found->visited = false;
-
        rwlock_unlock(&lock);
 }
 
index ed7dadbd7c722f369b0c522d24cd01a2c4afd046..74d87bb34d356a31ca25198e7a45efb71fb38105 100644 (file)
@@ -558,7 +558,7 @@ check_ancestor_error(struct rpki_uri *requested_uri)
  * validated the TA's public key.
  */
 int
-download_files(struct rpki_uri *requested_uri, bool is_ta, bool force)
+rsync_download_files(struct rpki_uri *requested_uri, bool is_ta, bool force)
 {
        /**
         * Note:
index 00b086b6c5343bc9f9761e45b401fe8df642d712..a19682be67f3fcddf6745b2e74100b5cfe257cac 100644 (file)
@@ -6,7 +6,7 @@
 
 struct uri_list;
 
-int download_files(struct rpki_uri *, bool, bool);
+int rsync_download_files(struct rpki_uri *, bool, bool);
 int rsync_create(struct uri_list **);
 void rsync_destroy(struct uri_list *);
 
index 1855325d72dbbc86d314cf98d5c7ad0965968818..4c9568f944cecefa93f9ed9a68614710671eb8ed 100644 (file)
 #include "slurm/slurm_loader.h"
 #include "thread/thread_pool.h"
 
-/*
- * Storage of VRPs (term taken from RFC 6811 "Validated ROA Payload") and
- * Serials that contain such VRPs
- */
-
 #define START_SERIAL           0
 
 DEFINE_ARRAY_LIST_FUNCTIONS(deltas_db, struct delta_group, )
@@ -302,7 +297,8 @@ __vrps_update(bool *changed)
        serial_t min_serial;
        int error;
 
-       *changed = false;
+       if (changed)
+               *changed = false;
        old_base = NULL;
        new_base = NULL;
 
@@ -366,7 +362,8 @@ __vrps_update(bool *changed)
                }
        }
 
-       *changed = true;
+       if (changed)
+               *changed = true;
        state.base = new_base;
        state.next_serial++;
 
index 7877084971dd814d6725918c332006a62ef01ab5..956b3d189808cd51520453b88dbace58e2da8c42 100644 (file)
@@ -1,6 +1,12 @@
 #ifndef SRC_VRPS_H_
 #define SRC_VRPS_H_
 
+/*
+ * "VRPs" = "Validated ROA Payloads." See RFC 6811.
+ *
+ * This module stores VRPs and their serials.
+ */
+
 #include <stdbool.h>
 #include "data_structure/array_list.h"
 #include "rtr/db/delta.h"
index 98b520bbe2c6b82e3a9f1db2b783c3110f4eb2dc..9678896a9c69c11f143c8368d335af8566781880 100644 (file)
@@ -16,7 +16,6 @@
 #include "clients.h"
 #include "internal_pool.h"
 #include "log.h"
-#include "validation_run.h"
 #include "rtr/err_pdu.h"
 #include "rtr/pdu.h"
 #include "rtr/db/vrps.h"
@@ -43,15 +42,12 @@ struct fd_node {
 /* List of server sockets */
 SLIST_HEAD(server_fds, fd_node);
 
+/* "file descriptors" */
+static struct server_fds fds;
+static struct thread_pool *threads;
 /* Does the server needs to be stopped? */
 static volatile bool server_stop;
 
-/* Parameters for the RTR server task */
-struct rtr_task_param {
-       struct server_fds *fds;
-       struct thread_pool *pool;
-};
-
 static int
 init_addrinfo(char const *hostname, char const *service,
     struct addrinfo **result)
@@ -112,7 +108,7 @@ create_server_socket(char const *hostname, char const *service, int *result)
        unsigned long port;
        int flags;
        int reuse;
-       int fd; /* "file descriptor" */
+       int fd;
        int error;
 
        *result = 0; /* Shuts up gcc */
@@ -210,7 +206,7 @@ fd_node_create(struct fd_node **result)
 }
 
 static int
-server_fd_add(struct server_fds *fds, char const *address, char const *service)
+server_fd_add(char const *address, char const *service)
 {
        struct fd_node *node;
        int error;
@@ -226,23 +222,22 @@ server_fd_add(struct server_fds *fds, char const *address, char const *service)
                return error;
        }
 
-       SLIST_INSERT_HEAD(fds, node, next);
+       SLIST_INSERT_HEAD(&fds, node, next);
        pr_op_debug("Created server socket with FD %d.", node->id);
        return 0;
 }
 
 static void
-server_fds_destroy(struct server_fds *fds)
+destroy_fds(void)
 {
        struct fd_node *fd;
 
-       while (!SLIST_EMPTY(fds)) {
-               fd = fds->slh_first;
-               SLIST_REMOVE_HEAD(fds, next);
+       while (!SLIST_EMPTY(&fds)) {
+               fd = fds.slh_first;
+               SLIST_REMOVE_HEAD(&fds, next);
                close(fd->id);
                free(fd);
        }
-       free(fds);
 }
 
 static int
@@ -294,7 +289,7 @@ parse_address(char const *full_address, char const *default_service,
 }
 
 static int
-create_server_sockets(struct server_fds *fds)
+init_fds(void)
 {
        struct string_array const *addresses;
        char const *default_service;
@@ -306,7 +301,7 @@ create_server_sockets(struct server_fds *fds)
        default_service = config_get_server_port();
        addresses = config_get_server_address();
        if (addresses->length == 0)
-               return server_fd_add(fds, NULL, default_service);
+               return server_fd_add(NULL, default_service);
 
        for (i = 0; i < addresses->length; i++) {
                address = NULL;
@@ -316,7 +311,7 @@ create_server_sockets(struct server_fds *fds)
                if (error)
                        return error;
 
-               error = server_fd_add(fds, address, service);
+               error = server_fd_add(address, service);
                /* Always release them */
                free(address);
                free(service);
@@ -491,9 +486,6 @@ init_fdset(struct server_fds *fds, fd_set *fdset)
 static void *
 handle_client_connections(void *arg)
 {
-       struct rtr_task_param *rtr_param = arg;
-       struct server_fds *fds;
-       struct thread_pool *pool;
        struct sockaddr_storage client_addr;
        struct thread_param *param;
        struct timeval select_time;
@@ -504,13 +496,7 @@ handle_client_connections(void *arg)
        int fd;
        int error;
 
-       /* Get the argument pointers, and release arg at once */
-       fds = rtr_param->fds;
-       pool = rtr_param->pool;
-       free(rtr_param);
-
-       last_server_fd = SLIST_FIRST(fds)->id;
-
+       last_server_fd = SLIST_FIRST(&fds)->id;
        sizeof_client_addr = sizeof(client_addr);
 
        /* I'm alive! */
@@ -526,7 +512,7 @@ handle_client_connections(void *arg)
                if (server_stop)
                        break;
 
-               init_fdset(fds, &readfds);
+               init_fdset(&fds, &readfds);
 
                if (select(last_server_fd + 1, &readfds, NULL, NULL,
                    &select_time) == -1) {
@@ -535,7 +521,7 @@ handle_client_connections(void *arg)
                }
 
                for (fd = 0; fd < (last_server_fd + 1); fd++) {
-                       if (!FD_ISSET (fd, &readfds))
+                       if (!FD_ISSET(fd, &readfds))
                                continue;
 
                        /* Accept the connection */
@@ -556,7 +542,7 @@ handle_client_connections(void *arg)
                         * client at the thread pool queue since it's probable
                         * that it'll remain there forever.
                         */
-                       if (!thread_pool_avail_threads(pool)) {
+                       if (!thread_pool_avail_threads(threads)) {
                                reject_client(client_fd, &client_addr);
                                continue;
                        }
@@ -581,7 +567,7 @@ handle_client_connections(void *arg)
                        param->fd = client_fd;
                        param->addr = client_addr;
 
-                       error = thread_pool_push(pool, "Client thread",
+                       error = thread_pool_push(threads, "Client thread",
                            client_thread_cb, param);
                        if (error) {
                                pr_op_err("Couldn't push a thread to attend incoming RTR client");
@@ -593,13 +579,12 @@ handle_client_connections(void *arg)
                }
        } while (true);
 
-       return NULL; /* Unreachable. */
+       return NULL;
 }
 
 static int
-__handle_client_connections(struct server_fds *fds, struct thread_pool *pool)
+start_server_thread(void)
 {
-       struct rtr_task_param *param;
        struct fd_node *node;
        struct sigaction ign;
        int error;
@@ -610,98 +595,56 @@ __handle_client_connections(struct server_fds *fds, struct thread_pool *pool)
        sigemptyset(&ign.sa_mask);
        sigaction(SIGPIPE, &ign, NULL);
 
-       SLIST_FOREACH(node, fds, next) {
+       SLIST_FOREACH(node, &fds, next) {
                error = listen(node->id, config_get_server_queue());
                if (error)
                        return pr_op_errno(errno,
                            "Couldn't listen on server socket");
        }
 
-       param = malloc(sizeof(struct rtr_task_param));
-       if (param == NULL)
-               return pr_enomem();
-
-       param->fds = fds;
-       param->pool = pool;
-
-       /* handle_client_connections() must release param */
-       error = internal_pool_push("Server thread", handle_client_connections,
-           param);
-       if (error) {
-               free(param);
-               return error;
-       }
-
-       return 0;
+       return internal_pool_push("Server thread", handle_client_connections,
+           NULL);
 }
 
 /*
- * Starts the server, using the current thread to listen for RTR client
- * requests. If configuration parameter 'mode' is STANDALONE, then the
- * server runs "one time" (a.k.a. run the validation just once), it doesn't
- * waits for clients requests.
- *
- * When listening for client requests, this function blocks.
+ * Starts the RTR server.
  */
 int
-rtr_listen(void)
+rtr_start(void)
 {
-       struct server_fds *fds; /* "file descriptors" */
-       struct thread_pool *pool;
        int error;
 
+       SLIST_INIT(&fds);
+       threads = NULL;
        server_stop = true;
 
-       error = clients_db_init();
-       if (error)
-               return error;
-
-       if (config_get_mode() == STANDALONE) {
-               error = validation_run_first();
-               goto revert_clients_db; /* Error 0 it's ok */
-       }
-
-       fds = malloc(sizeof(struct server_fds));
-       if (fds == NULL) {
-               error = pr_enomem();
-               goto revert_clients_db;
-       }
-
-       SLIST_INIT(fds);
-       error = create_server_sockets(fds);
+       error = init_fds();
        if (error)
                goto revert_server_fds;
 
-       pool = NULL;
        error = thread_pool_create("Server",
-           config_get_thread_pool_server_max(), &pool);
+           config_get_thread_pool_server_max(), &threads);
        if (error)
                goto revert_server_fds;
 
-       /* Do the first run */
-       error = validation_run_first();
+       error = start_server_thread();
        if (error)
                goto revert_thread_pool;
 
-       /* Wait for connections at another thread */
-       error = __handle_client_connections(fds, pool);
-       if (error)
-               goto revert_thread_pool;
-
-       /* Keep running the validations on the main thread */
-       error = validation_run_cycle();
-
-       /* Terminate all clients */
-       clients_terminate_all(end_client, CL_TERMINATED);
-
-       /* Stop the server (it lives on a detached thread) */
-       server_stop = true;
+       return 0;
 
 revert_thread_pool:
-       thread_pool_destroy(pool);
+       thread_pool_destroy(threads);
 revert_server_fds:
-       server_fds_destroy(fds);
-revert_clients_db:
-       clients_db_destroy();
+       destroy_fds();
        return error;
 }
+
+void
+rtr_stop(void)
+{
+       server_stop = true;
+       clients_terminate_all(end_client, CL_TERMINATED);
+       thread_pool_destroy(threads);
+       destroy_fds();
+}
index 7353be96508878469aafff27effb676f4ba2c23a..7922af52852139c28490433ed7bf1e6365c2086e 100644 (file)
@@ -1,6 +1,7 @@
 #ifndef RTR_RTR_H_
 #define RTR_RTR_H_
 
-int rtr_listen(void);
+int rtr_start(void);
+void rtr_stop(void);
 
 #endif /* RTR_RTR_H_ */
index 598e01a40a5df90527301cc0604ec7c8606b1b63..392515ba6558de6c8ca47d5eda0d69d60fe3a68e 100644 (file)
--- a/src/uri.c
+++ b/src/uri.c
@@ -18,8 +18,6 @@ static char const *const PFX_RSYNC = "rsync://";
 static char const *const PFX_HTTPS = "https://";
 
 /**
- * All rpki_uris are guaranteed to be RSYNC URLs right now.
- *
  * Design notes:
  *
  * Because we need to generate @local from @global, @global's allowed character
index 8e02612b60c5da27459be7cc073a9cec4a76c9f1..1efb9d7d4fd85551fbc02b651d9760af2bcde813 100644 (file)
 int
 validation_run_first(void)
 {
-       bool upd;
        int error;
 
        if (config_get_mode() == SERVER)
                pr_op_warn("First validation cycle has begun, wait until the next notification to connect your router(s)");
        else
-               pr_op_info("First validation cycle has begun");
+               pr_op_info("The validation has begun.");
 
-       upd = false;
-       error = vrps_update(&upd);
+       error = vrps_update(NULL);
        if (error)
                return pr_op_err("First validation wasn't successful.");
 
        if (config_get_mode() == SERVER)
                pr_op_warn("First validation cycle successfully ended, now you can connect your router(s)");
        else
-               pr_op_info("First validation cycle successfully ended, terminating execution");
+               pr_op_info("The validation has successfully ended.");
 
        return 0;
 }