From 70afe64a29294c04a95e019caa1e99d03489c757 Mon Sep 17 00:00:00 2001 From: Andy Alt Date: Wed, 20 Nov 2024 13:56:01 -0600 Subject: [PATCH] maint: Refactor trashinfo; cleanup (#492) * maint: Cleanup trashinfo * more cleanup, obliterate union * Add error-checking to fprintf when writing trashinfo file * combine enums * Move some vars into globals.c * rename function * narrow scope of LEN_DELETION_DATE_KEY_WITH_VALUE --- src/config_rmw.c | 3 +- src/globals.c | 3 + src/globals.h | 17 ++-- src/main.c | 2 +- src/messages.c | 3 +- src/purging.c | 2 +- src/restore.c | 2 +- src/trashinfo.c | 199 ++++++++++++++++++++++------------------------- src/trashinfo.h | 33 ++------ src/utils.c | 12 +-- test/meson.build | 2 +- 11 files changed, 126 insertions(+), 152 deletions(-) diff --git a/src/config_rmw.c b/src/config_rmw.c index c5b80349..06798f7a 100644 --- a/src/config_rmw.c +++ b/src/config_rmw.c @@ -28,7 +28,6 @@ along with this program. If not, see . // #include // for major() and minor() static const int DEFAULT_EXPIRE_AGE = 0; -static const char *lit_files = "files"; const char *expire_age_str = "expire_age"; @@ -243,7 +242,7 @@ parse_line_waste(st_waste *waste_curr, struct Canfigger *node, } /* and the files... */ - waste_curr->files = join_paths(waste_curr->parent, lit_files); + waste_curr->files = join_paths(waste_curr->parent, "files"); waste_curr->len_files = strlen(waste_curr->files); int p_state = check_pathname_state(waste_curr->files); diff --git a/src/globals.c b/src/globals.c index adfc744c..34308896 100644 --- a/src/globals.c +++ b/src/globals.c @@ -21,3 +21,6 @@ along with this program. If not, see . #include "globals.h" int verbose = 0; +const char *lit_info = "info"; +const char trashinfo_ext[] = ".trashinfo"; +const int len_trashinfo_ext = sizeof trashinfo_ext - 1; /* Subtract 1 for the terminating NULL */ diff --git a/src/globals.h b/src/globals.h index 521dd907..6f229b81 100644 --- a/src/globals.h +++ b/src/globals.h @@ -47,12 +47,19 @@ along with this program. If not, see . #define VERSION "unversioned" #endif -#ifndef PATH_MAX -#define PATH_MAX 4096 -#endif - -#define LEN_MAX_ESCAPED_PATH (((PATH_MAX - 1) * 3) + 1) +#define LEN_MAX_ESCAPED_PATH (PATH_MAX * 3) #define EBUF 11 +typedef enum +{ + TI_HEADER, + PATH_KEY, + DELETIONDATE_KEY, + TI_LINE_MAX +} ti_key; + extern int verbose; +extern const char *lit_info; +extern const char trashinfo_ext[]; +extern const int len_trashinfo_ext; diff --git a/src/main.c b/src/main.c index bcb0a8b1..1a3f2eec 100644 --- a/src/main.c +++ b/src/main.c @@ -680,7 +680,7 @@ main(const int argc, char *const argv[]) parse_cli_options(argc, argv, &cli_user_options); if (verbose > 1) - printf("PATH_MAX = %d\n", PATH_MAX - 1); + printf("PATH_MAX = %d\n", PATH_MAX); if (verbose > 0) #ifdef HAVE_LINUX_BTRFS diff --git a/src/messages.c b/src/messages.c index f2870873..b05162a5 100644 --- a/src/messages.c +++ b/src/messages.c @@ -94,8 +94,7 @@ display_dot_trashinfo_error(const char *dti) * "format" refers to the layout of the file * contents */ - printf(_("format of .trashinfo `file %s` is incorrect"), dti); - putchar('\n'); + printf(_("format of .trashinfo file '%s' is incorrect\n"), dti); return; } diff --git a/src/purging.c b/src/purging.c index e99726d8..26f4bad9 100644 --- a/src/purging.c +++ b/src/purging.c @@ -95,7 +95,7 @@ static time_t get_then_time(const char *tinfo_file) { char *raw_deletion_date = - parse_trashinfo_file(tinfo_file, deletion_date_key); + validate_and_get_value(tinfo_file, DELETIONDATE_KEY); if (raw_deletion_date != NULL) { struct tm tm_then; diff --git a/src/restore.c b/src/restore.c index 3efd91f9..56a1a1e4 100644 --- a/src/restore.c +++ b/src/restore.c @@ -112,7 +112,7 @@ restore(const char *src, st_time *st_time_var, tmp_str = NULL; sn_check(r, PATH_MAX); - char *_dest = parse_trashinfo_file(src_tinfo, path_key); + char *_dest = validate_and_get_value(src_tinfo, PATH_KEY); if (_dest == NULL) return -1; diff --git a/src/trashinfo.c b/src/trashinfo.c index f527977b..37187a2a 100644 --- a/src/trashinfo.c +++ b/src/trashinfo.c @@ -18,37 +18,17 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . */ +#include + #include "trashinfo.h" #include "utils.h" #include "messages.h" -#define TI_LINE_COUNT 3 - -enum -{ - TI_HEADER, - TI_PATH_LINE, - TI_DATE_LINE -}; - -static const char ti_header[] = "[Trash Info]"; -static const char ti_path[] = "Path="; -static const char ti_date[] = "DeletionDate="; +const size_t LEN_MAX_TRASHINFO_PATH_LINE = + sizeof "Path=" + LEN_MAX_ESCAPED_PATH - 1; -static const struct st__trashinfo st_trashinfo_template[] = { - {ti_header, sizeof ti_header - 1}, - {ti_path, sizeof ti_path - 1}, - {ti_date, sizeof ti_date - 1}, -}; - -const char trashinfo_ext[] = ".trashinfo"; -const int len_trashinfo_ext = sizeof trashinfo_ext - 1; /* Subtract 1 for the terminating NULL */ -const int LEN_MAX_TRASHINFO_PATH_LINE = - (sizeof ti_path - 1) + LEN_MAX_ESCAPED_PATH; - -const char *lit_info = "info"; -const char *path_key = "Path"; -const char *deletion_date_key = "DeletionDate"; +const struct trashinfo_template trashinfo_template = + { "[Trash Info]", "Path=", "DeletionDate=" }; int @@ -105,13 +85,32 @@ create_trashinfo(rmw_target *st_f_props, st_waste *waste_curr, } } - fprintf(fp, "%s\n", st_trashinfo_template[TI_HEADER].str); - fprintf(fp, "%s%s\n", st_trashinfo_template[TI_PATH_LINE].str, - escaped_path_ptr); + ssize_t want_size = strlen(trashinfo_template.header) + 1 + + strlen(trashinfo_template.path_key) + + strlen(escaped_path_ptr) + 1 + + strlen(trashinfo_template.deletion_date_key) + + strlen(st_time_var->deletion_date) + 1; + + int n = fprintf(fp, "%s\n%s%s\n%s%s\n", trashinfo_template.header, + trashinfo_template.path_key, escaped_path_ptr, + trashinfo_template.deletion_date_key, + st_time_var->deletion_date); + free(escaped_path); - fprintf(fp, "%s%s\n", st_trashinfo_template[TI_DATE_LINE].str, - st_time_var->deletion_date); + if (n < 0) + { + print_msg_error(); + fprintf(stderr, "fprintf() failed due to an error writing to %s\n", + final_info_dest); + } + else if (n != want_size) + { + print_msg_error(); + fprintf(stderr, + "Expected to write %zu bytes, but wrote %d bytes to %s\n", + want_size, n, final_info_dest); + } return close_file(&fp, final_info_dest, __func__); } else @@ -122,108 +121,96 @@ create_trashinfo(rmw_target *st_f_props, st_waste *waste_curr, } -/* - * name: parse_trashinfo_file - * - * Checks the integrity of a trashinfo file and returns req_value for - * either the Path or DeletionDate key - * - */ char * -parse_trashinfo_file(const char *file, const char *req_value) +validate_and_get_value(const char *file, ti_key key) { - struct trashinfo_field trashinfo_field; - if (strcmp(req_value, path_key) != 0 - && strcmp(req_value, deletion_date_key) != 0) + const uint8_t LEN_DELETION_DATE_KEY_WITH_VALUE = 32; + struct { - print_msg_error(); - fprintf(stderr, "Required arg for %s can be either \"%s\" or \"%s\".", - __func__, path_key, deletion_date_key); - return NULL; - } + bool header_ok; + bool path_ok; + bool date_ok; + } ti_status = { false, false, false }; - int line_no = 0; FILE *fp = fopen(file, "r"); if (fp != NULL) { - trashinfo_field.value = NULL; - bool res = true; + char *key_value = NULL; + uint8_t line_n = 0; + char fp_line[LEN_MAX_TRASHINFO_PATH_LINE]; while (fgets(fp_line, LEN_MAX_TRASHINFO_PATH_LINE, fp) != NULL - && res == true) + && line_n < TI_LINE_MAX) { trim_whitespace(fp_line); - - switch (line_no) + char *val_ptr; + switch (line_n) { case TI_HEADER: - res = - strncmp(fp_line, st_trashinfo_template[TI_HEADER].str, - st_trashinfo_template[TI_HEADER].len) == 0; + ti_status.header_ok = + (strcmp(fp_line, trashinfo_template.header) == 0); break; - case TI_PATH_LINE: - res = - strncmp(fp_line, st_trashinfo_template[TI_PATH_LINE].str, - st_trashinfo_template[TI_PATH_LINE].len) == 0; - if (res && strcmp(req_value, path_key) == 0) + case PATH_KEY: + ti_status.path_ok = + (strncmp + (fp_line, trashinfo_template.path_key, + strlen(trashinfo_template.path_key)) == 0); + if (ti_status.path_ok && key == PATH_KEY) { - trashinfo_field.f.path_ptr = strchr(fp_line, '='); - trashinfo_field.f.path_ptr++; /* move past the '=' sign */ - char *unescaped_path = unescape_url(trashinfo_field.f.path_ptr); - trashinfo_field.value = unescaped_path; + val_ptr = strchr(fp_line, '='); + if (val_ptr) + { + val_ptr++; /* move past the '=' sign */ + char *unescaped_path = unescape_url(val_ptr); + if (!unescaped_path) + fatal_malloc(); + key_value = unescaped_path; + } } break; - case TI_DATE_LINE: - res = - strncmp(fp_line, st_trashinfo_template[TI_DATE_LINE].str, - st_trashinfo_template[TI_DATE_LINE].len) == 0 - && strlen(fp_line) == 32; - - if (res && strcmp(req_value, deletion_date_key) == 0) + case DELETIONDATE_KEY: + ti_status.date_ok = + (strncmp(fp_line, trashinfo_template.deletion_date_key, + strlen(trashinfo_template.deletion_date_key)) == 0) + && strlen(fp_line) == LEN_DELETION_DATE_KEY_WITH_VALUE; + if (ti_status.date_ok && key == DELETIONDATE_KEY) { - trashinfo_field.f.date_str_ptr = strchr(fp_line, '='); - trashinfo_field.f.date_str_ptr++; - trashinfo_field.value = strdup(trashinfo_field.f.date_str_ptr); - if (!trashinfo_field.value) - fatal_malloc(); + val_ptr = strchr(fp_line, '='); + if (val_ptr) + { + val_ptr++; + key_value = strdup(val_ptr); + if (!key_value) + fatal_malloc(); + } } break; - default: - res = false; - break; } - line_no++; + line_n++; } close_file(&fp, file, __func__); - if (res && line_no == TI_LINE_COUNT) - return trashinfo_field.value; + if (ti_status.header_ok && ti_status.path_ok && ti_status.date_ok + && key_value) + return key_value; - if (trashinfo_field.value != NULL) - free(trashinfo_field.value); + if (key_value != NULL) + free(key_value); display_dot_trashinfo_error(file); return NULL; } - else - { - open_err(file, __func__); - return NULL; - } + open_err(file, __func__); + return NULL; } -/////////////////////////////////////////////////////////////////////// -#ifdef TEST_LIB - -#include "test.h" - -int -main() -{ - assert(strcmp(st_trashinfo_template[TI_HEADER].str, "[Trash Info]") == 0); - assert(strcmp(st_trashinfo_template[TI_PATH_LINE].str, "Path=") == 0); - assert(strcmp(st_trashinfo_template[TI_DATE_LINE].str, "DeletionDate=") == - 0); - - return 0; -} -#endif +//const char *ti_key_to_string(ti_key key) +//{ + //switch (key) + //{ + //case TI_HEADER: return "TI_HEADER"; + //case PATH_KEY: return "PATH_KEY"; + //case DELETIONDATE_KEY: return "DELETIONDATE_KEY"; + //case TI_LINE_MAX: return "TI_LINE_MAX"; + //default: return "UNKNOWN_KEY"; + //} +//} diff --git a/src/trashinfo.h b/src/trashinfo.h index 52144bee..342c8c8a 100644 --- a/src/trashinfo.h +++ b/src/trashinfo.h @@ -25,13 +25,6 @@ along with this program. If not, see . #include "time_rmw.h" -extern const char trashinfo_ext[]; -extern const int len_trashinfo_ext; - -extern const int TI_LINE_COUNT; - -extern const int LEN_MAX_TRASHINFO_PATH_LINE; - /** Each waste directory is added to a linked list and has the data * from this structure associated with it. */ @@ -103,30 +96,16 @@ typedef struct bool is_duplicate; } rmw_target; - -extern const char *lit_info; -extern const char *path_key; -extern const char *deletion_date_key; - -struct st__trashinfo +extern const struct trashinfo_template { - const char *str; - const unsigned short int len; -}; - + const char *header; + const char *path_key; + const char *deletion_date_key; +} trashinfo_template; -struct trashinfo_field -{ - char *value; - union - { - char *path_ptr; - char *date_str_ptr; - } f; -}; int create_trashinfo(rmw_target * st_f_props, st_waste * waste_curr, st_time * st_time_var); -char *parse_trashinfo_file(const char *file, const char *req_value); +char *validate_and_get_value(const char *file, ti_key key); diff --git a/src/utils.c b/src/utils.c index 16897f73..03a342e8 100644 --- a/src/utils.c +++ b/src/utils.c @@ -146,17 +146,17 @@ check_pathname_state(const char *pathname) int fd = open(pathname, O_RDONLY | O_NOFOLLOW); if (fd != -1) { - close(fd); + if (close(fd) != 0) + fprintf(stderr, "close: %s\n%s", pathname, strerror(errno)); return EEXIST; } /* FreeBSD sets errno to EMLINK instead of ELOOP as specified by POSIX - when O_NOFOLLOW is set in flags and the final component of pathname is - a symbolic link to distinguish it from the case of too many symbolic - link traversals in one of its non-final components. - https://man.freebsd.org/cgi/man.cgi?query=open + when O_NOFOLLOW is set in flags and the final component of pathname is + a symbolic link to distinguish it from the case of too many symbolic + link traversals in one of its non-final components. + https://man.freebsd.org/cgi/man.cgi?query=open */ - if (errno == ELOOP || errno == EMLINK) return EEXIST; else if (errno == ENOENT) diff --git a/test/meson.build b/test/meson.build index 05f0786f..670952e4 100644 --- a/test/meson.build +++ b/test/meson.build @@ -9,7 +9,7 @@ if faketime.found() ) endif -test_cases = ['strings_rmw', 'utils', 'trashinfo', 'restore'] +test_cases = ['strings_rmw', 'utils', 'restore'] scripts = [ 'test_basic.sh',