From: Alberto Leiva Popper Date: Wed, 29 Dec 2021 19:22:09 +0000 (-0600) Subject: Patch a bunch of easy TODOs X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=8e073c2973552381594930fc802454853a899d97;p=thirdparty%2FFORT-validator.git Patch a bunch of easy TODOs --- diff --git a/src/common.c b/src/common.c index 9c4dd99a..23dea6ad 100644 --- a/src/common.c +++ b/src/common.c @@ -1,15 +1,5 @@ #include "common.h" -#include /* readdir(), closedir() */ -#include /* realpath() */ -#include /* malloc(), free(), realloc(), realpath() */ -#include /* remove() */ -#include /* strdup(), strrchr(), strcmp(), strcat(), etc */ -#include /* stat(), rmdir() */ -#include /* stat(), mkdir() */ -#include /* stat(), closedir(), mkdir() */ - -#include "config.h" #include "log.h" int @@ -67,306 +57,6 @@ rwlock_unlock(pthread_rwlock_t *lock) error); } -static int -process_file(char const *dir_name, char const *file_name, char const *file_ext, - int *fcount, process_file_cb cb, void *arg) -{ - char *ext, *fullpath, *tmp; - int error; - - if (file_ext != NULL) { - ext = strrchr(file_name, '.'); - /* Ignore file if extension isn't the expected */ - if (ext == NULL || strcmp(ext, file_ext) != 0) - return 0; - } - - (*fcount)++; /* Increment the found count */ - - /* Get the full file path */ - tmp = strdup(dir_name); - if (tmp == NULL) - return -pr_op_errno(errno, "Couldn't create temporal char"); - - tmp = realloc(tmp, strlen(tmp) + 1 + strlen(file_name) + 1); - if (tmp == NULL) - return -pr_op_errno(errno, "Couldn't reallocate temporal char"); - - strcat(tmp, "/"); - strcat(tmp, file_name); - fullpath = realpath(tmp, NULL); - if (fullpath == NULL) { - free(tmp); - return -pr_op_errno(errno, - "Error getting real path for file '%s' at dir '%s'", - dir_name, file_name); - } - - error = cb(fullpath, arg); - free(fullpath); - free(tmp); - return error; -} - -static int -process_dir_files(char const *location, char const *file_ext, bool empty_err, - process_file_cb cb, void *arg) -{ - DIR *dir_loc; - struct dirent *dir_ent; - int found, error; - - dir_loc = opendir(location); - if (dir_loc == NULL) { - error = -pr_op_errno(errno, "Couldn't open dir %s", location); - goto end; - } - - errno = 0; - found = 0; - while ((dir_ent = readdir(dir_loc)) != NULL) { - error = process_file(location, dir_ent->d_name, file_ext, - &found, cb, arg); - if (error) { - pr_op_err("The error was at file %s", dir_ent->d_name); - goto close_dir; - } - errno = 0; - } - if (errno) { - pr_op_err("Error reading dir %s", location); - error = -errno; - } - if (!error && found == 0) - error = (empty_err ? - pr_op_err("Location '%s' doesn't have files with extension '%s'", - location, file_ext) : - pr_op_warn("Location '%s' doesn't have files with extension '%s'", - location, file_ext)); - -close_dir: - closedir(dir_loc); -end: - return error; -} - -int -process_file_or_dir(char const *location, char const *file_ext, bool empty_err, - process_file_cb cb, void *arg) -{ - struct stat attr; - int error; - - error = stat(location, &attr); - if (error) - return pr_op_errno(errno, "Error reading path '%s'", location); - - if (S_ISDIR(attr.st_mode) == 0) - return cb(location, arg); - - return process_dir_files(location, file_ext, empty_err, cb, arg); -} - - -bool -valid_file_or_dir(char const *location, bool check_file, bool check_dir, - int (*error_fn)(int error, const char *format, ...)) -{ - struct stat attr; - bool is_file, is_dir; - bool result; - - if (!check_file && !check_dir) - pr_crit("Wrong usage, at least one check must be 'true'."); - - if (stat(location, &attr) == -1) { - if (error_fn != NULL) { - error_fn(errno, "stat(%s) failed: %s", location, - strerror(errno)); - } - return false; - } - - is_file = check_file && S_ISREG(attr.st_mode); - is_dir = check_dir && S_ISDIR(attr.st_mode); - - result = is_file || is_dir; - if (!result) - pr_op_err("'%s' does not seem to be a %s", location, - (check_file && check_dir) ? "file or directory" : - (check_file) ? "file" : "directory"); - - return result; -} - -static int -dir_exists(char const *path, bool *result) -{ - struct stat _stat; - char *last_slash; - - last_slash = strrchr(path, '/'); - if (last_slash == NULL) { - /* - * Simply because create_dir_recursive() has nothing meaningful - * to do when this happens. It's a pretty strange error. - */ - *result = true; - return 0; - } - - *last_slash = '\0'; - - if (stat(path, &_stat) == 0) { - if (!S_ISDIR(_stat.st_mode)) { - return pr_op_err("Path '%s' exists and is not a directory.", - path); - } - *result = true; - } else if (errno == ENOENT) { - *result = false; - } else { - return pr_op_errno(errno, "stat() failed"); - } - - *last_slash = '/'; - return 0; -} - -static int -create_dir(char *path) -{ - int error; - - error = mkdir(path, 0777); - - if (error && errno != EEXIST) - return pr_op_errno(errno, "Error while making directory '%s'", - path); - - return 0; -} - -/** - * Apparently, RSYNC does not like to create parent directories. - * This function fixes that. - * - * TODO (aaaa) that's not enough. Document this properly. - * TODO (aaaa) maybe move this to file.c? - */ -int -create_dir_recursive(char const *path) -{ - char *localuri; - int i, error; - bool exist = false; - - error = dir_exists(path, &exist); - if (error) - return error; - if (exist) - return 0; - - localuri = strdup(path); - if (localuri == NULL) - return pr_enomem(); - - for (i = 1; localuri[i] != '\0'; i++) { - if (localuri[i] == '/') { - localuri[i] = '\0'; - error = create_dir(localuri); - localuri[i] = '/'; - if (error) { - /* error msg already printed */ - free(localuri); - return error; - } - } - } - - free(localuri); - return 0; -} - -static int -remove_file(char const *path) -{ - int error; - - errno = 0; - error = remove(path); - if (error) - return pr_val_errno(errno, "Couldn't delete %s", path); - - return 0; -} - -/* - * Delete parent dirs of @path only if dirs are empty, @path must be a file - * location and will be deleted first. - * - * The algorithm is a bit aggressive, but rmdir() won't delete - * something unless is empty, so in case the dir still has something in - * it the cycle is finished. - * - * TODO (aaaa) move to file.c? - */ -int -delete_dir_recursive_bottom_up(char const *path) -{ - char *config_repo; - char *work_loc, *tmp; - size_t config_len; - int error; - - error = remove_file(path); - if (error) - return error; - - config_repo = strdup(config_get_local_repository()); - if (config_repo == NULL) - return pr_enomem(); - - /* Stop dir removal when the work_dir has this length */ - config_len = strlen(config_repo); - if (config_repo[config_len - 1] == '/') - config_len--; - free(config_repo); - - work_loc = strdup(path); - if (work_loc == NULL) - return pr_enomem(); - - do { - tmp = strrchr(work_loc, '/'); - if (tmp == NULL) - break; - *tmp = '\0'; - - /* Stop if the root dir is reached */ - if (strlen(work_loc) == config_len) - break; - - errno = 0; - error = rmdir(work_loc); - if (!error) - continue; /* Keep deleting up */ - - /* Stop if there's content in the dir */ - if (errno == ENOTEMPTY || errno == EEXIST) - break; - - error = pr_op_errno(errno, "Couldn't delete dir %s", work_loc); - goto release_str; - } while (true); - - free(work_loc); - return 0; -release_str: - free(work_loc); - return error; -} - int get_current_time(time_t *result) { diff --git a/src/common.h b/src/common.h index 23962a3d..ac5a2f9b 100644 --- a/src/common.h +++ b/src/common.h @@ -2,19 +2,11 @@ #define SRC_RTR_COMMON_H_ #include -#include #include /* "I think that this is not supposed to be implemented." */ #define ENOTSUPPORTED 3172 -/* - * A request made to a server (eg. rsync, http) has failed, even after retrying - * - * TODO (aaaa) I think this should just be EAGAIN. - */ -#define EREQFAILED 3176 - /* * If you're wondering why I'm not using -abs(error), it's because abs(INT_MIN) * overflows, so gcc complains sometimes. @@ -33,16 +25,6 @@ int rwlock_read_lock(pthread_rwlock_t *); void rwlock_write_lock(pthread_rwlock_t *); void rwlock_unlock(pthread_rwlock_t *); -typedef int (*process_file_cb)(char const *, void *); -int process_file_or_dir(char const *, char const *, bool, process_file_cb, - void *); - -typedef int (*pr_errno_cb)(int, const char *, ...); -bool valid_file_or_dir(char const *, bool, bool, pr_errno_cb); - -int create_dir_recursive(char const *); -int delete_dir_recursive_bottom_up(char const *); - int get_current_time(time_t *); #endif /* SRC_RTR_COMMON_H_ */ diff --git a/src/crypto/base64.c b/src/crypto/base64.c index f839bc78..4820800a 100644 --- a/src/crypto/base64.c +++ b/src/crypto/base64.c @@ -5,7 +5,9 @@ #include #include #include + #include "log.h" +#include "str_token.h" /** * Converts error from libcrypto representation to this project's @@ -207,6 +209,7 @@ to_base64url(char *base, size_t base_len, char **out) char *pad, *tmp; size_t len; int i; + int error; /* Remove padding, if present */ len = base_len; @@ -217,12 +220,9 @@ to_base64url(char *base, size_t base_len, char **out) len = pad - base; } while(0); - tmp = malloc(len + 1); - if (tmp == NULL) - return pr_enomem(); - - memcpy(tmp, base, len); - tmp[len] = '\0'; + error = string_clone(base, len, &tmp); + if (error) + return error; for (i = 0; i < len; i++) { if (tmp[i] == '+') diff --git a/src/data_structure/path_builder.c b/src/data_structure/path_builder.c index 9412908b..51c6d342 100644 --- a/src/data_structure/path_builder.c +++ b/src/data_structure/path_builder.c @@ -67,13 +67,10 @@ path_append_limited(struct path_builder *pb, char const *addend, } void -path_append_url(struct path_builder *pb, struct rpki_uri *uri) +path_append_url(struct path_builder *pb, char const *guri) { - char const *guri; char *colon; - guri = uri_get_global(uri); - /* Is there really a point to removing the colon? */ colon = strchr(guri, ':'); if (colon != NULL) { diff --git a/src/data_structure/path_builder.h b/src/data_structure/path_builder.h index 67dc872c..79a80068 100644 --- a/src/data_structure/path_builder.h +++ b/src/data_structure/path_builder.h @@ -1,10 +1,7 @@ #ifndef SRC_DATA_STRUCTURE_PATH_BUILDER_H_ #define SRC_DATA_STRUCTURE_PATH_BUILDER_H_ -/* TODO (aaaa) needs unit tests */ - #include -#include "types/uri.h" struct path_builder { char *string; @@ -17,7 +14,7 @@ void path_init(struct path_builder *); void path_append(struct path_builder *, char const *); void path_append_limited(struct path_builder *, char const *, size_t); -void path_append_url(struct path_builder *, struct rpki_uri *); +void path_append_url(struct path_builder *, char const *); int path_compile(struct path_builder *, char **); diff --git a/src/file.c b/src/file.c index fe031e59..cdf7e80a 100644 --- a/src/file.c +++ b/src/file.c @@ -1,9 +1,11 @@ #include "file.h" -#include -#include -#include -#include +#include /* readdir(), closedir() */ +#include /* realpath() */ +#include /* malloc(), free(), realloc(), realpath() */ +#include /* strdup(), strrchr(), strcmp(), strcat(), etc */ + +#include "config.h" #include "log.h" static int @@ -164,3 +166,299 @@ file_get_modification_time(char const *luri) return metadata.st_mtim.tv_sec; } + +static int +process_file(char const *dir_name, char const *file_name, char const *file_ext, + int *fcount, process_file_cb cb, void *arg) +{ + char *ext, *fullpath, *tmp; + int error; + + if (file_ext != NULL) { + ext = strrchr(file_name, '.'); + /* Ignore file if extension isn't the expected */ + if (ext == NULL || strcmp(ext, file_ext) != 0) + return 0; + } + + (*fcount)++; /* Increment the found count */ + + /* Get the full file path */ + tmp = strdup(dir_name); + if (tmp == NULL) + return -pr_op_errno(errno, "Couldn't create temporal char"); + + tmp = realloc(tmp, strlen(tmp) + 1 + strlen(file_name) + 1); + if (tmp == NULL) + return -pr_op_errno(errno, "Couldn't reallocate temporal char"); + + strcat(tmp, "/"); + strcat(tmp, file_name); + fullpath = realpath(tmp, NULL); + if (fullpath == NULL) { + free(tmp); + return -pr_op_errno(errno, + "Error getting real path for file '%s' at dir '%s'", + dir_name, file_name); + } + + error = cb(fullpath, arg); + free(fullpath); + free(tmp); + return error; +} + +static int +process_dir_files(char const *location, char const *file_ext, bool empty_err, + process_file_cb cb, void *arg) +{ + DIR *dir_loc; + struct dirent *dir_ent; + int found, error; + + dir_loc = opendir(location); + if (dir_loc == NULL) { + error = -pr_op_errno(errno, "Couldn't open dir %s", location); + goto end; + } + + errno = 0; + found = 0; + while ((dir_ent = readdir(dir_loc)) != NULL) { + error = process_file(location, dir_ent->d_name, file_ext, + &found, cb, arg); + if (error) { + pr_op_err("The error was at file %s", dir_ent->d_name); + goto close_dir; + } + errno = 0; + } + if (errno) { + pr_op_err("Error reading dir %s", location); + error = -errno; + } + if (!error && found == 0) + error = (empty_err ? + pr_op_err("Location '%s' doesn't have files with extension '%s'", + location, file_ext) : + pr_op_warn("Location '%s' doesn't have files with extension '%s'", + location, file_ext)); + +close_dir: + closedir(dir_loc); +end: + return error; +} + +int +process_file_or_dir(char const *location, char const *file_ext, bool empty_err, + process_file_cb cb, void *arg) +{ + struct stat attr; + int error; + + error = stat(location, &attr); + if (error) + return pr_op_errno(errno, "Error reading path '%s'", location); + + if (S_ISDIR(attr.st_mode) == 0) + return cb(location, arg); + + return process_dir_files(location, file_ext, empty_err, cb, arg); +} + + +bool +valid_file_or_dir(char const *location, bool check_file, bool check_dir, + int (*error_fn)(int error, const char *format, ...)) +{ + struct stat attr; + bool is_file, is_dir; + bool result; + + if (!check_file && !check_dir) + pr_crit("Wrong usage, at least one check must be 'true'."); + + if (stat(location, &attr) == -1) { + if (error_fn != NULL) { + error_fn(errno, "stat(%s) failed: %s", location, + strerror(errno)); + } + return false; + } + + is_file = check_file && S_ISREG(attr.st_mode); + is_dir = check_dir && S_ISDIR(attr.st_mode); + + result = is_file || is_dir; + if (!result) + pr_op_err("'%s' does not seem to be a %s", location, + (check_file && check_dir) ? "file or directory" : + (check_file) ? "file" : "directory"); + + return result; +} + +static int +dir_exists(char const *path, bool *result) +{ + struct stat _stat; + char *last_slash; + + last_slash = strrchr(path, '/'); + if (last_slash == NULL) { + /* + * Simply because create_dir_recursive() has nothing meaningful + * to do when this happens. It's a pretty strange error. + */ + *result = true; + return 0; + } + + *last_slash = '\0'; + + if (stat(path, &_stat) == 0) { + if (!S_ISDIR(_stat.st_mode)) { + return pr_op_err("Path '%s' exists and is not a directory.", + path); + } + *result = true; + } else if (errno == ENOENT) { + *result = false; + } else { + return pr_op_errno(errno, "stat() failed"); + } + + *last_slash = '/'; + return 0; +} + +static int +create_dir(char *path) +{ + int error; + + error = mkdir(path, 0777); + + if (error && errno != EEXIST) + return pr_op_errno(errno, "Error while making directory '%s'", + path); + + return 0; +} + +/** + * Ensures all the ancestor directories of @path exist. + * + * eg. if @path is "/a/b/c/d.txt", creates a, b and c (if they don't exist). + */ +int +create_dir_recursive(char const *path) +{ + char *localuri; + int i, error; + bool exist = false; + + error = dir_exists(path, &exist); + if (error) + return error; + if (exist) + return 0; + + localuri = strdup(path); + if (localuri == NULL) + return pr_enomem(); + + for (i = 1; localuri[i] != '\0'; i++) { + if (localuri[i] == '/') { + localuri[i] = '\0'; + error = create_dir(localuri); + localuri[i] = '/'; + if (error) { + /* error msg already printed */ + free(localuri); + return error; + } + } + } + + free(localuri); + return 0; +} + +static int +remove_file(char const *path) +{ + int error; + + errno = 0; + error = remove(path); + if (error) + return pr_val_errno(errno, "Couldn't delete %s", path); + + return 0; +} + +/* + * Delete parent dirs of @path only if dirs are empty, @path must be a file + * location and will be deleted first. + * + * The algorithm is a bit aggressive, but rmdir() won't delete + * something unless is empty, so in case the dir still has something in + * it the cycle is finished. + */ +int +delete_dir_recursive_bottom_up(char const *path) +{ + char *config_repo; + char *work_loc, *tmp; + size_t config_len; + int error; + + error = remove_file(path); + if (error) + return error; + + config_repo = strdup(config_get_local_repository()); + if (config_repo == NULL) + return pr_enomem(); + + /* Stop dir removal when the work_dir has this length */ + config_len = strlen(config_repo); + if (config_repo[config_len - 1] == '/') + config_len--; + free(config_repo); + + work_loc = strdup(path); + if (work_loc == NULL) + return pr_enomem(); + + do { + tmp = strrchr(work_loc, '/'); + if (tmp == NULL) + break; + *tmp = '\0'; + + /* Stop if the root dir is reached */ + if (strlen(work_loc) == config_len) + break; + + errno = 0; + error = rmdir(work_loc); + if (!error) + continue; /* Keep deleting up */ + + /* Stop if there's content in the dir */ + if (errno == ENOTEMPTY || errno == EEXIST) + break; + + error = pr_op_errno(errno, "Couldn't delete dir %s", work_loc); + goto release_str; + } while (true); + + free(work_loc); + return 0; +release_str: + free(work_loc); + return error; +} diff --git a/src/file.h b/src/file.h index 95443387..67a869f3 100644 --- a/src/file.h +++ b/src/file.h @@ -2,9 +2,10 @@ #define SRC_FILE_H_ #include -#include -#include -#include +#include /* FILE, remove() */ +#include /* stat, closedir(), mkdir() */ +#include /* stat, mkdir() */ +#include /* stat(), rmdir() */ /* * The entire contents of the file, loaded into a buffer. @@ -26,4 +27,14 @@ void file_free(struct file_contents *); bool file_valid(char const *); long file_get_modification_time(char const *); +typedef int (*process_file_cb)(char const *, void *); +int process_file_or_dir(char const *, char const *, bool, process_file_cb, + void *); + +typedef int (*pr_errno_cb)(int, const char *, ...); +bool valid_file_or_dir(char const *, bool, bool, pr_errno_cb); + +int create_dir_recursive(char const *); +int delete_dir_recursive_bottom_up(char const *); + #endif /* SRC_FILE_H_ */ diff --git a/src/http/http.c b/src/http/http.c index f3ce16b0..d9daddd6 100644 --- a/src/http/http.c +++ b/src/http/http.c @@ -262,7 +262,7 @@ do_single_http_get(struct curl_args *handler, char const *src, char const *dst) case CURLE_COULDNT_RESOLVE_HOST: case CURLE_COULDNT_RESOLVE_PROXY: case CURLE_FTP_ACCEPT_TIMEOUT: - return EREQFAILED; /* Retry */ + return EAGAIN; /* Retry */ default: return handle_http_response_code(http_code); } @@ -361,8 +361,10 @@ http_get(struct rpki_uri *uri, long ims) char *tmp_file; int error; - if (!config_get_http_enabled()) + if (!config_get_http_enabled()) { + pr_val_debug("HTTP disabled; skipping download."); return ENOTCHANGED; + } /* TODO (aaaa) this is reusable. Move to the thread. */ error = http_easy_init(&handler, ims); @@ -402,8 +404,6 @@ free_handler: /* * Downloads @remote to the absolute path @dest (no workspace nor directory * structure is created). - * - * TODO (aaaa) this function needs to shrink even more. */ int http_direct_download(char const *remote, char const *dest) @@ -418,6 +418,5 @@ http_direct_download(char const *remote, char const *dest) error = do_single_http_get(&curl, remote, dest); http_easy_cleanup(&curl); - return error; } diff --git a/src/object/certificate.c b/src/object/certificate.c index 418118e0..1fad192d 100644 --- a/src/object/certificate.c +++ b/src/object/certificate.c @@ -37,8 +37,6 @@ * The X509V3_EXT_METHOD that references NID_sinfo_access uses the AIA item. * The SIA's d2i function, therefore, returns AIAs. * They are the same as far as LibreSSL is concerned. - * - * TODO (aaaa) still needed? */ typedef AUTHORITY_INFO_ACCESS SIGNATURE_INFO_ACCESS; @@ -241,7 +239,7 @@ check_dup_public_key(bool *duplicated, char const *file, void *arg) rcvd_cert = NULL; tmp_size = 0; - /* TODO (aaaa) what about RRDP? */ + /* what about RRDP? */ /* error = uri_create_rsync(&uri, file); if (error) @@ -319,6 +317,7 @@ validate_subject(X509 *cert) return error; pr_val_debug("Subject: %s", x509_name_commonName(name)); + /* TODO (aaaa) */ if (false) { error = x509stack_store_subject(validation_certstack(state_retrieve()), name, check_dup_public_key, cert); @@ -1436,7 +1435,6 @@ end: static int asnstr2str(ASN1_STRING *asnstr, char **result) { - char *str; int str_len; /* @@ -1449,15 +1447,7 @@ asnstr2str(ASN1_STRING *asnstr, char **result) if (str_len < 0) return pr_val_err("String has invalid length: %d", str_len); - str = malloc(str_len + 1); - if (str == NULL) - return pr_enomem(); - - memcpy(str, ASN1_STRING_get0_data(asnstr), str_len); - str[str_len] = '\0'; - - *result = str; - return 0; + return string_clone(ASN1_STRING_get0_data(asnstr), str_len, result); } /* Create @uri from @ad */ diff --git a/src/object/name.c b/src/object/name.c index 2ebcce98..d644f11d 100644 --- a/src/object/name.c +++ b/src/object/name.c @@ -7,6 +7,7 @@ #include "log.h" #include "thread_var.h" +#include "str_token.h" /** * It's an RFC5280 name, but from RFC 6487's perspective. @@ -21,24 +22,15 @@ struct rfc5280_name { }; static int -name2string(X509_NAME_ENTRY *name, char **_result) +name2string(X509_NAME_ENTRY *name, char **result) { const ASN1_STRING *data; - char *result; data = X509_NAME_ENTRY_get_data(name); if (data == NULL) return val_crypto_err("X509_NAME_ENTRY_get_data() returned NULL"); - result = malloc(data->length + 1); - if (result == NULL) - return pr_enomem(); - - memcpy(result, data->data, data->length); - result[data->length] = '\0'; - - *_result = result; - return 0; + return string_clone(data->data, data->length, result); } int diff --git a/src/object/tal.c b/src/object/tal.c index 4e95d5b8..fbdcf656 100644 --- a/src/object/tal.c +++ b/src/object/tal.c @@ -16,6 +16,7 @@ #include "types/uri_list.h" #include "crypto/base64.h" #include "rtr/db/vrps.h" +#include "thread/thread_pool.h" typedef int (*foreach_uri_cb)(struct tal *, struct rpki_uri *, void *); diff --git a/src/rpp/rpp_dl_status.c b/src/rpp/rpp_dl_status.c index 224a4c5c..01197de4 100644 --- a/src/rpp/rpp_dl_status.c +++ b/src/rpp/rpp_dl_status.c @@ -79,7 +79,6 @@ rdsdb_set(struct rpki_uri *uri, int error) char const *key; size_t key_len; - /* TODO (aaaa) check both protocols do it like this. */ if (error == ENOTCHANGED) { pr_val_debug("No updates."); error = 0; diff --git a/src/rpp/rpp_dl_status.h b/src/rpp/rpp_dl_status.h index 6a987b6e..a7253318 100644 --- a/src/rpp/rpp_dl_status.h +++ b/src/rpp/rpp_dl_status.h @@ -21,7 +21,6 @@ enum rpp_download_status { */ struct rpp_dl_status_db; -/* TODO (aaaa) call these */ struct rpp_dl_status_db *rdsdb_create(void); void rdsdb_destroy(struct rpp_dl_status_db *); diff --git a/src/rrdp/types.c b/src/rrdp/types.c index 0e8a4ce6..1d7e40d6 100644 --- a/src/rrdp/types.c +++ b/src/rrdp/types.c @@ -5,6 +5,7 @@ #include "common.h" #include "file.h" #include "log.h" +#include "str_token.h" #include "crypto/base64.h" #include "crypto/hash.h" #include "xml/relax_ng.h" @@ -209,15 +210,13 @@ release_sanitized: return error; } -/* - * TODO (aaaa) at least one caller doesn't benefit from the memory copy. - * Analyze. - */ static int parse_string(xmlTextReaderPtr reader, char const *attr, char **result) { xmlChar *xml_value; - char *tmp; + int error; + + *result = NULL; if (attr == NULL) { xml_value = xmlTextReaderValue(reader); @@ -231,18 +230,9 @@ parse_string(xmlTextReaderPtr reader, char const *attr, char **result) attr, xmlTextReaderConstLocalName(reader)); } - tmp = malloc(xmlStrlen(xml_value) + 1); - if (tmp == NULL) { - xmlFree(xml_value); - return pr_enomem(); - } - - memcpy(tmp, xml_value, xmlStrlen(xml_value)); - tmp[xmlStrlen(xml_value)] = '\0'; + error = string_clone(xml_value, xmlStrlen(xml_value), result); xmlFree(xml_value); - - *result = tmp; - return 0; + return error; } static int diff --git a/src/rsync/rsync.c b/src/rsync/rsync.c index 63b40058..b4eec0c7 100644 --- a/src/rsync/rsync.c +++ b/src/rsync/rsync.c @@ -10,6 +10,7 @@ #include "common.h" #include "log.h" +#include "str_token.h" #include "rpp/rpp_dl_status.h" /* @@ -122,13 +123,11 @@ log_buffer(char const *buffer, ssize_t read, int type) { #define PRE_RSYNC "[RSYNC exec]: " char *cpy, *cur, *tmp; + int error; - cpy = malloc(read + 1); - if (cpy == NULL) - return pr_enomem(); - - strncpy(cpy, buffer, read); - cpy[read] = '\0'; + error = string_clone(buffer, read, &cpy); + if (error) + return error; /* Break lines to one line at log */ cur = cpy; @@ -299,7 +298,7 @@ do_rsync(struct rpki_uri *uri, bool is_file) if (retries > 0) pr_val_warn("Max RSYNC retries (%u) reached on '%s', won't retry again.", retries, uri_get_global(uri)); - error = EREQFAILED; + error = -EINVAL; goto release_args; } pr_val_warn("Retrying RSYNC '%s' in %u seconds, %u attempts remaining.", @@ -353,9 +352,8 @@ rsync_download_files(struct rpki_uri *uri, bool is_file) int error; if (!config_get_rsync_enabled()) { - /* TODO (aaaa) equivalent RRDP message */ pr_val_debug("rsync disabled; skipping update."); - return 0; + return ENOTCHANGED; } switch (rdsdb_get(uri)) { diff --git a/src/rtr/rtr.c b/src/rtr/rtr.c index 606f1db6..aebd6211 100644 --- a/src/rtr/rtr.c +++ b/src/rtr/rtr.c @@ -13,6 +13,7 @@ #include #include "config.h" +#include "str_token.h" #include "types/address.h" #include "data_structure/array_list.h" #include "rtr/pdu.h" @@ -100,7 +101,7 @@ parse_address(char const *full_address, char **address, char **service) char *ptr; char *tmp_addr; char *tmp_serv; - size_t tmp_addr_len; + int error; if (full_address == NULL) { tmp_addr = NULL; @@ -129,13 +130,10 @@ parse_address(char const *full_address, char **address, char **service) return pr_op_err("Invalid server address '%s', can't end with '#'", full_address); - tmp_addr_len = strlen(full_address) - strlen(ptr); - tmp_addr = malloc(tmp_addr_len + 1); - if (tmp_addr == NULL) - return pr_enomem(); - - memcpy(tmp_addr, full_address, tmp_addr_len); - tmp_addr[tmp_addr_len] = '\0'; + error = string_clone(full_address, strlen(full_address) - strlen(ptr), + &tmp_addr); + if (error) + return error; tmp_serv = strdup(ptr + 1); if (tmp_serv == NULL) { diff --git a/src/str_token.c b/src/str_token.c index bdf6f8ce..cb42688f 100644 --- a/src/str_token.c +++ b/src/str_token.c @@ -4,19 +4,17 @@ #include #include "log.h" -/** - * Does not assume that @string is NULL-terminated. - * - * TODO (aaaa) use this more. - */ -static int +/* Does not assume that @string is NULL-terminated. */ +int string_clone(void const *string, size_t size, char **clone) { char *result; result = malloc(size + 1); - if (result == NULL) + if (result == NULL) { + *clone = NULL; return pr_enomem(); + } memcpy(result, string, size); result[size] = '\0'; diff --git a/src/str_token.h b/src/str_token.h index 12da0fe4..00c2836c 100644 --- a/src/str_token.h +++ b/src/str_token.h @@ -6,6 +6,8 @@ #include #include +int string_clone(void const *, size_t, char **); + int ia5s2string(ASN1_IA5STRING *, char **); int BN2string(BIGNUM *, char **); diff --git a/src/types/uri.c b/src/types/uri.c index 4eae271a..5d5b74ef 100644 --- a/src/types/uri.c +++ b/src/types/uri.c @@ -97,7 +97,7 @@ uri_create(char *global, enum rpki_uri_type type, struct rpki_uri **result) case URI_TYPE_HTTP_SIMPLE: path_init(&path); path_append(&path, config_get_local_repository()); - path_append_url(&path, uri); /* Note: Must include the protocol */ + path_append_url(&path, uri_get_global(uri)); error = path_compile(&path, &uri->local); if (error) { uri_refput(uri); @@ -133,8 +133,8 @@ uri_create_caged(char *global, struct rpki_uri *notification, path_init(&path); path_append(&path, config_get_local_repository()); path_append(&path, "caged"); - path_append_url(&path, notification); - path_append_url(&path, uri); + path_append_url(&path, uri_get_global(notification)); + path_append_url(&path, uri_get_global(uri)); error = path_compile(&path, &uri->local); if (error) { uri_refput(uri); diff --git a/src/types/uri_list.c b/src/types/uri_list.c index bc16ac0e..3983dbdc 100644 --- a/src/types/uri_list.c +++ b/src/types/uri_list.c @@ -101,23 +101,23 @@ try_download(struct uri_list *uris, bool try_rrdp, bool try_rsync) struct rpki_uri *uri; char const *guri; size_t u; + int error; ARRAYLIST_FOREACH(uris, __uri, u) { uri = *__uri; guri = uri_get_global(uri); if (try_rrdp && is_http(guri)) { - if (uri_get_type(uri) == URI_TYPE_VERSATILE) { - if (http_update(uri) == 0) - return uri; - } else { - if (rrdp_update(uri) == 0) - return uri; - } + error = (uri_get_type(uri) == URI_TYPE_VERSATILE) + ? http_update(uri) + : rrdp_update(uri); + if (!error || error == ENOTCHANGED || error == EFBIG) + return uri; } if (try_rsync && is_rsync(guri)) { - if (rsync_download_files(uri, false) == 0) - return 0; + error = rsync_download_files(uri, false); + if (!error || error == ENOTCHANGED || error == EFBIG) + return uri; } } diff --git a/test/Makefile.am b/test/Makefile.am index 63a810e6..4fe572f0 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -23,8 +23,11 @@ check_PROGRAMS = address.test check_PROGRAMS += deltas_array.test check_PROGRAMS += db_table.test check_PROGRAMS += line_file.test +check_PROGRAMS += path_builder.test +check_PROGRAMS += pdu.test check_PROGRAMS += pdu_handler.test -check_PROGRAMS += rrdp_objects.test +check_PROGRAMS += primitive_reader.test +check_PROGRAMS += rrdp/notification.test check_PROGRAMS += serial.test check_PROGRAMS += tal.test check_PROGRAMS += thread_pool.test @@ -32,8 +35,6 @@ check_PROGRAMS += uri.test check_PROGRAMS += vcard.test check_PROGRAMS += vrps.test check_PROGRAMS += xml.test -check_PROGRAMS += rtr/pdu.test -check_PROGRAMS += rtr/primitive_reader.test TESTS = ${check_PROGRAMS} address_test_SOURCES = types/address_test.c @@ -48,11 +49,20 @@ db_table_test_LDADD = ${MY_LDADD} line_file_test_SOURCES = line_file_test.c line_file_test_LDADD = ${MY_LDADD} +path_builder_test_SOURCES = data_structure/path_builder_test.c +path_builder_test_LDADD = ${MY_LDADD} + +pdu_test_SOURCES = rtr/pdu_test.c +pdu_test_LDADD = ${MY_LDADD} + pdu_handler_test_SOURCES = rtr/pdu_handler_test.c pdu_handler_test_LDADD = ${MY_LDADD} ${JANSSON_LIBS} -rrdp_objects_test_SOURCES = rrdp_objects_test.c -rrdp_objects_test_LDADD = ${MY_LDADD} ${JANSSON_LIBS} ${XML2_LIBS} +primitive_reader_test_SOURCES = rtr/primitive_reader_test.c +primitive_reader_test_LDADD = ${MY_LDADD} + +rrdp_notification_test_SOURCES = rrdp/notification_test.c +rrdp_notification_test_LDADD = ${MY_LDADD} ${JANSSON_LIBS} ${XML2_LIBS} serial_test_SOURCES = types/serial_test.c serial_test_LDADD = ${MY_LDADD} @@ -75,12 +85,6 @@ vrps_test_LDADD = ${MY_LDADD} ${JANSSON_LIBS} xml_test_SOURCES = xml_test.c xml_test_LDADD = ${MY_LDADD} ${XML2_LIBS} -rtr_pdu_test_SOURCES = rtr/pdu_test.c -rtr_pdu_test_LDADD = ${MY_LDADD} - -rtr_primitive_reader_test_SOURCES = rtr/primitive_reader_test.c -rtr_primitive_reader_test_LDADD = ${MY_LDADD} - EXTRA_DIST = impersonator.c EXTRA_DIST += line_file/core.txt EXTRA_DIST += line_file/empty.txt diff --git a/test/data_structure/path_builder_test.c b/test/data_structure/path_builder_test.c new file mode 100644 index 00000000..897fce3f --- /dev/null +++ b/test/data_structure/path_builder_test.c @@ -0,0 +1,99 @@ +#include "data_structure/path_builder.c" + +#include + +#include "impersonator.c" +#include "log.c" + +static void +validate_pb(struct path_builder *pb, char const *expected) +{ + char *path; + ck_assert_int_eq(0, path_compile(pb, &path)); + ck_assert_str_eq(expected, path); + free(path); +} + +START_TEST(path_append_test) +{ + struct path_builder pb; + + path_init(&pb); + validate_pb(&pb, ""); + + path_init(&pb); + path_append(&pb, ""); + path_append(&pb, ""); + validate_pb(&pb, ""); + + path_init(&pb); + path_append(&pb, "a"); + validate_pb(&pb, "a"); + + path_init(&pb); + path_append(&pb, "a"); + path_append(&pb, "b"); + path_append(&pb, "c"); + validate_pb(&pb, "a/b/c"); + + path_init(&pb); + path_append(&pb, "a/b"); + path_append(&pb, "c"); + validate_pb(&pb, "a/b/c"); +} +END_TEST + +START_TEST(path_append_url_test) +{ + struct path_builder pb; + + path_init(&pb); + path_append_url(&pb, "http://a/b/c.txt"); + validate_pb(&pb, "http/a/b/c.txt"); + + path_init(&pb); + path_append_url(&pb, "rsync://a/b/c.txt"); + path_append_url(&pb, "http://d/e/f.txt"); + validate_pb(&pb, "rsync/a/b/c.txt/http/d/e/f.txt"); + + path_init(&pb); + path_append_url(&pb, "abcdef"); + validate_pb(&pb, "abcdef"); + + path_init(&pb); + path_append_url(&pb, "abcdef"); + path_append_url(&pb, "rsync://a/b/c.txt"); + path_append_url(&pb, "http://d/e/f.txt"); + validate_pb(&pb, "abcdef/rsync/a/b/c.txt/http/d/e/f.txt"); +} +END_TEST + +Suite *path_builder_suite(void) +{ + Suite *suite; + TCase *core; + + core = tcase_create("Core"); + tcase_add_test(core, path_append_test); + tcase_add_test(core, path_append_url_test); + + suite = suite_create("lfile_read()"); + suite_add_tcase(suite, core); + return suite; +} + +int main(void) +{ + Suite *suite; + SRunner *runner; + int tests_failed; + + suite = path_builder_suite(); + + runner = srunner_create(suite); + srunner_run_all(runner, CK_NORMAL); + tests_failed = srunner_ntests_failed(runner); + srunner_free(runner); + + return (tests_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE; +} diff --git a/test/line_file_test.c b/test/line_file_test.c index 4a37b614..7fa33841 100644 --- a/test/line_file_test.c +++ b/test/line_file_test.c @@ -1,10 +1,11 @@ +#include "line_file.c" + #include #include #include #include "file.c" #include "impersonator.c" -#include "line_file.c" #include "log.c" START_TEST(file_line_normal) diff --git a/test/rrdp_objects_test.c b/test/rrdp/notification_test.c similarity index 97% rename from test/rrdp_objects_test.c rename to test/rrdp/notification_test.c index 6a98face..30337573 100644 --- a/test/rrdp_objects_test.c +++ b/test/rrdp/notification_test.c @@ -1,10 +1,11 @@ +#include "rrdp/notification.c" + #include #include #include #include "impersonator.c" #include "log.c" -#include "rrdp/notification.c" #include "rrdp/types.c" #include "types/uri.c" #include "data_structure/path_builder.c" @@ -55,6 +56,12 @@ base64_decode(BIO *in, unsigned char *out, bool has_nl, size_t out_len, pr_crit("Not supposed to be called."); } +int +string_clone(void const *string, size_t size, char **clone) +{ + pr_crit("Not supposed to be called."); +} + static void add_serials(struct rrdp_notification_deltas *deltas, ...) { diff --git a/test/rtr/db/vrps_test.c b/test/rtr/db/vrps_test.c index d96b1eec..5e74c3ee 100644 --- a/test/rtr/db/vrps_test.c +++ b/test/rtr/db/vrps_test.c @@ -24,6 +24,12 @@ #include "slurm/slurm_parser.c" #include "thread/thread_pool.c" +int +string_clone(void const *string, size_t size, char **clone) +{ + pr_crit("Not supposed to be called."); +} + /* -- Expected database descriptors -- */ /* diff --git a/test/rtr/pdu_handler_test.c b/test/rtr/pdu_handler_test.c index 29694df7..2d1f0b55 100644 --- a/test/rtr/pdu_handler_test.c +++ b/test/rtr/pdu_handler_test.c @@ -39,6 +39,12 @@ struct expected_pdu { static STAILQ_HEAD(, expected_pdu) expected_pdus = STAILQ_HEAD_INITIALIZER(expected_pdus); +int +string_clone(void const *string, size_t size, char **clone) +{ + pr_crit("Not supposed to be called."); +} + static void expected_pdu_add(uint8_t pdu_type) { diff --git a/test/tal_test.c b/test/tal_test.c index 4e590190..e8cc6d54 100644 --- a/test/tal_test.c +++ b/test/tal_test.c @@ -59,13 +59,6 @@ validation_destroy(struct validation *state) pr_crit("Not supposed to be called."); } -int -process_file_or_dir(char const *location, char const *file_ext, bool empty_err, - process_file_cb cb, void *arg) -{ - pr_crit("Not supposed to be called."); -} - void fnstack_init(void) { @@ -96,6 +89,19 @@ thread_pool_wait(struct thread_pool *pool) pr_crit("Not supposed to be called."); } +int +thread_pool_push(struct thread_pool *pool, char const *task_name, + thread_pool_task_cb cb, void *arg) +{ + pr_crit("Not supposed to be called."); +} + +int +string_clone(void const *string, size_t size, char **clone) +{ + pr_crit("Not supposed to be called."); +} + START_TEST(tal_load_normal) { struct tal *tal; diff --git a/test/xml_test.c b/test/xml_test.c index 0cbd1930..889fb5f4 100644 --- a/test/xml_test.c +++ b/test/xml_test.c @@ -5,6 +5,7 @@ #include #include "impersonator.c" #include "log.c" +#include "str_token.c" #include "xml/relax_ng.c" struct reader_ctx { @@ -17,34 +18,27 @@ static int reader_cb(xmlTextReaderPtr reader, void *arg) { struct reader_ctx *ctx = arg; - xmlReaderTypes type; xmlChar const *name; - xmlChar *tmp_char; - char *tmp; + xmlChar *serial; + int error; name = xmlTextReaderConstLocalName(reader); - type = xmlTextReaderNodeType(reader); - switch (type) { + switch (xmlTextReaderNodeType(reader)) { case XML_READER_TYPE_ELEMENT: if (xmlStrEqual(name, BAD_CAST "delta")) { ctx->delta_count++; } else if (xmlStrEqual(name, BAD_CAST "snapshot")) { ctx->snapshot_count++; } else if (xmlStrEqual(name, BAD_CAST "notification")) { - tmp_char = xmlTextReaderGetAttribute(reader, + serial = xmlTextReaderGetAttribute(reader, BAD_CAST "serial"); - if (tmp_char == NULL) + if (serial == NULL) return -EINVAL; - tmp = malloc(xmlStrlen(tmp_char) + 1); - if (tmp == NULL) { - xmlFree(tmp_char); - return -ENOMEM; - } - - memcpy(tmp, tmp_char, xmlStrlen(tmp_char)); - tmp[xmlStrlen(tmp_char)] = '\0'; - xmlFree(tmp_char); - ctx->serial = tmp; + error = string_clone(serial, xmlStrlen(serial), + &ctx->serial); + xmlFree(serial); + if (error) + return error; } else { return -EINVAL; }