From 236ca0102388bf5f12b4160dfef622a2706cd570 Mon Sep 17 00:00:00 2001 From: pcarana Date: Fri, 22 Mar 2019 12:07:04 -0600 Subject: [PATCH] Use warn(x) to log, don't start server without conf file --- src/address.c | 28 ++++------------------------ src/array_list.h | 4 ++-- src/clients.c | 2 +- src/configuration.c | 19 +++++++------------ src/csv.c | 15 +++++++-------- src/line_file.c | 10 ++++------ src/notify.c | 2 +- src/rtr/err_pdu.c | 2 +- src/rtr/pdu_handler.c | 12 +++++------- src/rtr/pdu_sender.c | 7 +++---- src/rtr/rtr.c | 6 ++++-- src/updates_daemon.c | 2 +- src/vrps.c | 4 ++-- 13 files changed, 42 insertions(+), 71 deletions(-) diff --git a/src/address.c b/src/address.c index 63639d13..de8b9fdf 100644 --- a/src/address.c +++ b/src/address.c @@ -103,16 +103,6 @@ prefix6_decode(const char *str, struct ipv6_prefix *result) { int error; - /* - * TODO (review) I see this pattern often: You call `err()`, and then - * throw a pretty exception. - * - * `err()` does not return, so any cleanup that follows it is pointless. - * - * More importantly, returnless functions are bad practice, - * because they destroy the recovery potential of calling code. - * Use `warn()`/`warnx()` instead (and continue throwing cleanly). - */ if (str == NULL) { warnx("Null string received, can't decode IPv6 prefix"); return -EINVAL; @@ -132,15 +122,6 @@ prefix_length_decode (const char *text, unsigned int *dst, int max_value) { unsigned long len; - /* - * TODO (review) You probably meant to use `errx()`, not `err()`. - * - * `err()` depends on a thread variable called `errno`. It makes no - * sense to call `err` when you know `errno` has not been set properly - * by some system function. - * - * If you haven't done it, see `man 3 err` and `man 3 errno`. - */ if (text == NULL) { warnx("Null string received, can't decode prefix length"); return -EINVAL; @@ -149,13 +130,12 @@ prefix_length_decode (const char *text, unsigned int *dst, int max_value) errno = 0; len = strtoul(text, NULL, 10); if (errno) { - warn("Invalid prefix length '%s': %s", text, - strerror(errno)); + warn("Invalid prefix length '%s'", text); return -EINVAL; } /* An underflow or overflow will be considered here */ if (len < 0 || max_value < len) { - warnx("Prefix length (%ld) is out of bounds (0-%d).", + warnx("Prefix length (%ld) is out of range (0-%d).", len, max_value); return -EINVAL; } @@ -169,7 +149,7 @@ prefix4_validate (struct ipv4_prefix *prefix) char buffer[INET_ADDRSTRLEN]; if ((prefix->addr.s_addr & be32_suffix_mask(prefix->len)) != 0) { - warn("IPv4 prefix %s/%u has enabled suffix bits.", + warnx("IPv4 prefix %s/%u has enabled suffix bits.", addr2str4(&prefix->addr, buffer), prefix->len); return -EINVAL; } @@ -188,7 +168,7 @@ prefix6_validate (struct ipv6_prefix *prefix) || (prefix->addr.s6_addr32[1] & suffix.s6_addr32[1]) || (prefix->addr.s6_addr32[2] & suffix.s6_addr32[2]) || (prefix->addr.s6_addr32[3] & suffix.s6_addr32[3])) { - warn("IPv6 prefix %s/%u has enabled suffix bits.", + warnx("IPv6 prefix %s/%u has enabled suffix bits.", addr2str6(&prefix->addr, buffer), prefix->len); return -EINVAL; } diff --git a/src/array_list.h b/src/array_list.h index 6232a6de..882fb52f 100644 --- a/src/array_list.h +++ b/src/array_list.h @@ -47,8 +47,8 @@ tmp = realloc(list->array, list->capacity \ * sizeof(elem_type)); \ if (tmp == NULL) { \ - err(-ENOMEM, "Out of memory"); \ - return -ENOMEM; \ + warn("Couldn't realloc array"); \ + return errno; \ } \ list->array = tmp; \ } \ diff --git a/src/clients.c b/src/clients.c index 28316e4f..c0fc6fce 100644 --- a/src/clients.c +++ b/src/clients.c @@ -16,7 +16,7 @@ clients_db_init(void) error = clientsdb_init(&clients_db); if (error) - err(error, "Clients DB couldn't be initialized"); + warnx( "Clients DB couldn't be initialized"); return error; } diff --git a/src/configuration.c b/src/configuration.c index 21748471..aa821442 100644 --- a/src/configuration.c +++ b/src/configuration.c @@ -74,12 +74,8 @@ config_init(char const *json_file_path) json_error_t json_error; int error; - /* - * TODO What's the point of a default start if there's - * no vrps input? - */ if (json_file_path == NULL) - return init_addrinfo(DEFAULT_ADDR, DEFAULT_PORT); + return -EINVAL; json_root = json_load_file(json_file_path, JSON_REJECT_DUPLICATES, &json_error); @@ -114,13 +110,13 @@ load_range(json_t *parent, char const *name, int default_value, error = json_get_int(parent, name, default_value, result); if (error) { - err(error, "Invalid value for '%s'", name); + warnx("Invalid value for '%s'", name); return error; } if (*result < min_value || max_value < *result) { - err(-EINVAL, "'%s' (%d) out of range, must be from %d to %d", - name, *result, min_value, max_value); + warnx("'%s' (%d) out of range, must be from %d to %d", name, + *result, min_value, max_value); return -EINVAL; } @@ -194,8 +190,8 @@ handle_json(json_t *root) config.vrps_location = strdup(vrps_location); if (config.vrps_location == NULL) { - err(errno, "'%s' couldn't be allocated.", - OPTNAME_VRPS_LOCATION); + warn("'%s' couldn't be allocated.", + OPTNAME_VRPS_LOCATION); return errno; } @@ -318,10 +314,9 @@ init_addrinfo(char const *hostname, char const *service) return error; } - /* TODO (review) check NULL */ config.port = strdup(service); if (config.port == NULL) { - err(errno, "'%s' couldn't be allocated.", OPTNAME_LISTEN_PORT); + warn( "'%s' couldn't be allocated.", OPTNAME_LISTEN_PORT); return errno; } diff --git a/src/csv.c b/src/csv.c index e464476b..ea89100e 100644 --- a/src/csv.c +++ b/src/csv.c @@ -39,7 +39,7 @@ parse_asn(char *text, unsigned int *value) } /* An underflow or overflow will be considered here */ if (asn < 0 || UINT32_MAX < asn) { - warnx("Prefix length (%lu) is out of bounds (0-%u).", + warnx("Prefix length (%lu) is out of range (0-%u).", asn, UINT32_MAX); return -EINVAL; } @@ -90,8 +90,8 @@ add_vrp(char *line, struct vrplist *vrplist) line_copy = malloc(strlen(line) + 1); if (line_copy == NULL) { - error = -ENOMEM; - err(error, "Out of memory allocating CSV line copy"); + error = errno; + warn("Out of memory allocating CSV line copy"); goto error; } strcpy(line_copy, line); @@ -143,8 +143,7 @@ add_vrp(char *line, struct vrplist *vrplist) if (prefix_length > max_prefix_length) { error = -EINVAL; - err(error, - "Prefix length is greater than max prefix length at line '%s'", + warnx("Prefix length is greater than max prefix length at line '%s'", line); goto error; } @@ -179,15 +178,15 @@ load_vrps(struct line_file *lfile, bool is_update) /* Init delta */ error = vrplist_init(&localvrps); - if (error ) { - warn("Couldn't allocate new VRPs"); + if (error) { + warnx("Couldn't allocate new VRPs"); return error; } current_line = 1; do { error = lfile_read(lfile, &line); if (error) { - warn("Error reading line %d, stop processing file.", + warnx("Error reading line %d, stop processing file.", current_line); goto end; } diff --git a/src/line_file.c b/src/line_file.c index a6f00d4c..923fe0ab 100644 --- a/src/line_file.c +++ b/src/line_file.c @@ -41,7 +41,7 @@ void lfile_close(struct line_file *lf) { if (fclose(lf->file) == -1) - err(errno, "fclose() failed: %s", strerror(errno)); + warn("fclose() failed"); free(lf); } @@ -103,7 +103,7 @@ lfile_read(struct line_file *lfile, char **result) free(string); *result = NULL; if (ferror(lfile->file)) { - err(error, "Error while reading file: %s\n", + warnx("Error while reading file: %s", strerror(error)); return error; } @@ -111,8 +111,7 @@ lfile_read(struct line_file *lfile, char **result) return 0; error = -EINVAL; - err(error, - "Supposedly unreachable code reached. ferror:%d feof:%d\n", + warnx("Supposedly unreachable code reached. ferror:%d feof:%d", ferror(lfile->file), feof(lfile->file)); return error; } @@ -127,8 +126,7 @@ lfile_read(struct line_file *lfile, char **result) for (i = 0; i < len; i++) { if (string[i] == '\0') { error = -EINVAL; - err(error, - "File '%s' has an illegal null character in its body. Please remove it.\n", + warnx("File '%s' has an illegal null character in its body. Please remove it.", lfile_name(lfile)); free(string); return error; diff --git a/src/notify.c b/src/notify.c index 25456feb..9d22aeaf 100644 --- a/src/notify.c +++ b/src/notify.c @@ -32,6 +32,6 @@ notify_clients(void) error = send_notify(ptr->fd, ptr->rtr_version); /* Error? Log it */ if (error) - err(error, "Error sending notify PDU"); + warnx("Error sending notify PDU to client"); } } diff --git a/src/rtr/err_pdu.c b/src/rtr/err_pdu.c index 8a809a22..cbea8177 100644 --- a/src/rtr/err_pdu.c +++ b/src/rtr/err_pdu.c @@ -66,6 +66,6 @@ err_pdu_log(u_int16_t code, char *message) break; } - warnx("Error report info: '%s', message '%s'.", + warnx("Error report PDU info: '%s', message '%s'.", code_title, message == NULL ? "[empty]" : message); } diff --git a/src/rtr/pdu_handler.c b/src/rtr/pdu_handler.c index 250d0be6..f60d08ee 100644 --- a/src/rtr/pdu_handler.c +++ b/src/rtr/pdu_handler.c @@ -91,9 +91,8 @@ handle_serial_query_pdu(int fd, void *pdu) return error; return send_end_of_data_pdu(&common); default: - error = -EINVAL; - err(error, "Reached 'unreachable' code"); - return error; + warnx("Reached 'unreachable' code"); + return -EINVAL; } } @@ -105,7 +104,7 @@ handle_reset_query_pdu(int fd, void *pdu) u_int32_t current_serial; u_int16_t session_id; u_int8_t version; - int error, updates; + int updates; version = received->header.protocol_version; session_id = get_current_session_id(version); @@ -123,9 +122,8 @@ handle_reset_query_pdu(int fd, void *pdu) /* https://tools.ietf.org/html/rfc8210#section-8.1 */ return send_commmon_exchange(&common); default: - error = -EINVAL; - err(error, "Reached 'unreachable' code"); - return error; + warnx("Reached 'unreachable' code"); + return -EINVAL; } } diff --git a/src/rtr/pdu_sender.c b/src/rtr/pdu_sender.c index 3cf291ea..34d28b41 100644 --- a/src/rtr/pdu_sender.c +++ b/src/rtr/pdu_sender.c @@ -85,9 +85,8 @@ send_response(int fd, char *data, size_t data_len) init_buffer(&buffer); /* Check for buffer overflow */ if (data_len > buffer.capacity) { - error = -EINVAL; - err(error, "Buffer out of capacity"); - return error; + warnx("Response buffer out of capacity"); + return -EINVAL; } memcpy(buffer.data, data, data_len); buffer.len = data_len; @@ -95,7 +94,7 @@ send_response(int fd, char *data, size_t data_len) error = write(fd, buffer.data, buffer.len); free_buffer(&buffer); if (error < 0) { - err(errno, "Error sending response"); + warnx("Error sending response"); return error; } diff --git a/src/rtr/rtr.c b/src/rtr/rtr.c index 99883ba7..7a1da410 100644 --- a/src/rtr/rtr.c +++ b/src/rtr/rtr.c @@ -133,6 +133,7 @@ client_thread_cb(void *param_void) err_pdu_send(param.client_fd, RTR_VERSION_SUPPORTED, ERR_PDU_UNSUP_PROTO_VERSION, (struct pdu_header *) pdu, NULL); + meta->destructor(pdu); return NULL; } /* RTR Version ready, now update client */ @@ -146,6 +147,7 @@ client_thread_cb(void *param_void) : ERR_PDU_UNEXPECTED_PROTO_VERSION), (struct pdu_header *) pdu, NULL); } + meta->destructor(pdu); return NULL; } @@ -194,8 +196,8 @@ handle_client_connections(int server_fd) */ arg = malloc(sizeof(struct thread_param)); - if (!arg) { - warnx("Thread parameter allocation failure"); + if (arg == NULL) { + warn("Thread parameter allocation failure"); continue; } arg->client_fd = client_fd; diff --git a/src/updates_daemon.c b/src/updates_daemon.c index fd4efbd6..f105c47d 100644 --- a/src/updates_daemon.c +++ b/src/updates_daemon.c @@ -18,7 +18,7 @@ check_vrps_updates(void *param_void) { updated = false; error = csv_check_vrps_file(&updated); if (error) { - err(error, "Error while searching CSV updates"); + warnx("Error while searching CSV updates, sleeping.."); goto sleep; } if (updated) diff --git a/src/vrps.c b/src/vrps.c index 71889406..8c8eb521 100644 --- a/src/vrps.c +++ b/src/vrps.c @@ -230,7 +230,7 @@ copy_vrps(struct vrp **dst, struct vrp *src, unsigned int len) struct vrp *tmp; tmp = realloc(*dst, len * sizeof(struct vrp)); if (tmp == NULL) { - warnx("Couldn't copy VRPs"); + warn("Couldn't copy VRPs"); return; } *dst = tmp; @@ -245,7 +245,7 @@ deltas_db_create_delta(struct vrp *array, unsigned int len) error = delta_init(&new_delta); if (error) { - warn("New Delta couldn't be initialized"); + warnx("New Delta couldn't be initialized"); return error; } -- 2.47.3