From 3d7a722ab2d120f898bf031a6217f22ff983f36a Mon Sep 17 00:00:00 2001 From: Andrea Mazzoleni Date: Sat, 3 Oct 2026 17:00:24 +0200 Subject: [PATCH] Avoid redundant strlen and duplicate traversals when loading content strings --- cmdline/elem.c | 18 ++++++++-------- cmdline/elem.h | 54 +++++++++++++++++++++++++++++++++++++++++------- cmdline/memory.c | 10 +++++++++ cmdline/memory.h | 6 ++++++ cmdline/state.c | 36 ++++++++++++++++++++++++-------- 5 files changed, 99 insertions(+), 25 deletions(-) diff --git a/cmdline/elem.c b/cmdline/elem.c index 5d9f1108..2401c755 100644 --- a/cmdline/elem.c +++ b/cmdline/elem.c @@ -470,7 +470,7 @@ int filter_snapshot(int enable, const char* sub, const char* name) return 0; } -struct snapraid_file* file_alloc(unsigned block_size, const char* sub, data_off_t size, uint64_t mtime_sec, int mtime_nsec, uint64_t inode) +struct snapraid_file* file_alloc_len(unsigned block_size, const char* sub, size_t sub_len, data_off_t size, uint64_t mtime_sec, int mtime_nsec, uint64_t inode) { struct snapraid_file* file; block_off_t blockmax; @@ -489,7 +489,7 @@ struct snapraid_file* file_alloc(unsigned block_size, const char* sub, data_off_ } file = malloc_nofail(sizeof(struct snapraid_file)); - file->sub = strdup_nofail(sub); + file->sub = strndup_nofail(sub, sub_len); file->size = size; file->blockmax = blockmax; file->mtime_sec = mtime_sec; @@ -794,13 +794,13 @@ int extent_file_compare(const void* void_a, const void* void_b) return 0; } -struct snapraid_link* link_alloc(const char* sub, const char* linkto, unsigned link_flag) +struct snapraid_link* link_alloc_len(const char* sub, size_t sub_len, const char* linkto, size_t linkto_len, unsigned link_flag) { struct snapraid_link* slink; slink = malloc_nofail(sizeof(struct snapraid_link)); - slink->sub = strdup_nofail(sub); - slink->linkto = strdup_nofail(linkto); + slink->sub = strndup_nofail(sub, sub_len); + slink->linkto = strndup_nofail(linkto, linkto_len); slink->flag = link_flag; return slink; @@ -831,12 +831,12 @@ int link_alpha_compare(const void* void_a, const void* void_b) return strcmp(slink_a->sub, slink_b->sub); } -struct snapraid_dir* dir_alloc(const char* sub) +struct snapraid_dir* dir_alloc_len(const char* sub, size_t sub_len) { struct snapraid_dir* dir; dir = malloc_nofail(sizeof(struct snapraid_dir)); - dir->sub = strdup_nofail(sub); + dir->sub = strndup_nofail(sub, sub_len); dir->flag = 0; return dir; @@ -858,7 +858,7 @@ int dir_name_compare(const void* void_arg, const void* void_data) return strcmp(arg, dir->sub); } -struct snapraid_dealloc* dealloc_alloc(unsigned block_size, const char* sub, data_off_t size, int64_t mtime_sec, int32_t mtime_nsec) +struct snapraid_dealloc* dealloc_alloc_len(unsigned block_size, const char* sub, size_t sub_len, data_off_t size, int64_t mtime_sec, int32_t mtime_nsec) { struct snapraid_dealloc* dealloc; block_off_t blockmax; @@ -877,7 +877,7 @@ struct snapraid_dealloc* dealloc_alloc(unsigned block_size, const char* sub, dat } dealloc = malloc_nofail(sizeof(struct snapraid_dealloc) + blockmax * sizeof(struct snapraid_dealloc_block)); - dealloc->sub = strdup_nofail(sub); + dealloc->sub = strndup_nofail(sub, sub_len); dealloc->size = size; dealloc->mtime_sec = mtime_sec; dealloc->mtime_nsec = mtime_nsec; diff --git a/cmdline/elem.h b/cmdline/elem.h index 81dd62a7..a507ba58 100644 --- a/cmdline/elem.h +++ b/cmdline/elem.h @@ -908,7 +908,11 @@ static inline void file_flag_clear(struct snapraid_file* file, unsigned mask) /** * Allocate a file without initializing its blocks. */ -struct snapraid_file* file_alloc(unsigned block_size, const char* sub, data_off_t size, uint64_t mtime_sec, int mtime_nsec, uint64_t inode); +struct snapraid_file* file_alloc_len(unsigned block_size, const char* sub, size_t sub_len, data_off_t size, uint64_t mtime_sec, int mtime_nsec, uint64_t inode); +static inline struct snapraid_file* file_alloc(unsigned block_size, const char* sub, data_off_t size, uint64_t mtime_sec, int mtime_nsec, uint64_t inode) +{ + return file_alloc_len(block_size, sub, strlen(sub), size, mtime_sec, mtime_nsec, inode); +} /** * Duplicate a file. @@ -994,12 +998,20 @@ int file_stamp_compare(const void* void_a, const void* void_b); int file_namestamp_compare(const void* void_a, const void* void_b); int file_pathstamp_compare(const void* void_a, const void* void_b); +/** + * Compute the hash of a file path with known length. + */ +static inline tommy_uint32_t file_path_hash_len(const char* sub, size_t len) +{ + return tommy_hash_u32(0, sub, len); +} + /** * Compute the hash of a file path. */ static inline tommy_uint32_t file_path_hash(const char* sub) { - return tommy_hash_u32(0, sub, strlen(sub)); + return file_path_hash_len(sub, strlen(sub)); } /** @@ -1059,7 +1071,11 @@ static inline unsigned link_flag_get(struct snapraid_link* slink, unsigned mask) /** * Allocate a link. */ -struct snapraid_link* link_alloc(const char* name, const char* slink, unsigned link_flag); +struct snapraid_link* link_alloc_len(const char* name, size_t name_len, const char* slink, size_t slink_len, unsigned link_flag); +static inline struct snapraid_link* link_alloc(const char* name, const char* slink, unsigned link_flag) +{ + return link_alloc_len(name, strlen(name), slink, strlen(slink), link_flag); +} /** * Deallocate a link. @@ -1076,12 +1092,20 @@ int link_name_compare_to_arg(const void* void_arg, const void* void_data); */ int link_alpha_compare(const void* void_a, const void* void_b); +/** + * Compute the hash of a link name with known length. + */ +static inline tommy_uint32_t link_name_hash_len(const char* name, size_t len) +{ + return tommy_hash_u32(0, name, len); +} + /** * Compute the hash of a link name. */ static inline tommy_uint32_t link_name_hash(const char* name) { - return tommy_hash_u32(0, name, strlen(name)); + return link_name_hash_len(name, strlen(name)); } static inline int dir_flag_has(const struct snapraid_dir* dir, unsigned mask) @@ -1102,7 +1126,11 @@ static inline void dir_flag_clear(struct snapraid_dir* dir, unsigned mask) /** * Allocate a dir. */ -struct snapraid_dir* dir_alloc(const char* name); +struct snapraid_dir* dir_alloc_len(const char* name, size_t name_len); +static inline struct snapraid_dir* dir_alloc(const char* name) +{ + return dir_alloc_len(name, strlen(name)); +} /** * Deallocate a dir. @@ -1114,18 +1142,30 @@ void dir_free(void* void_dir); */ int dir_name_compare(const void* void_arg, const void* void_data); +/** + * Compute the hash of a dir name with known length. + */ +static inline tommy_uint32_t dir_name_hash_len(const char* name, size_t len) +{ + return tommy_hash_u32(0, name, len); +} + /** * Compute the hash of a dir name. */ static inline tommy_uint32_t dir_name_hash(const char* name) { - return tommy_hash_u32(0, name, strlen(name)); + return dir_name_hash_len(name, strlen(name)); } /** * Allocate a dealloc. */ -struct snapraid_dealloc* dealloc_alloc(unsigned block_size, const char* sub, data_off_t size, int64_t mtime_sec, int32_t mtime_nsec); +struct snapraid_dealloc* dealloc_alloc_len(unsigned block_size, const char* sub, size_t sub_len, data_off_t size, int64_t mtime_sec, int32_t mtime_nsec); +static inline struct snapraid_dealloc* dealloc_alloc(unsigned block_size, const char* sub, data_off_t size, int64_t mtime_sec, int32_t mtime_nsec) +{ + return dealloc_alloc_len(block_size, sub, strlen(sub), size, mtime_sec, mtime_nsec); +} /** * Deallocate a dealloc. diff --git a/cmdline/memory.c b/cmdline/memory.c index 3354e2d3..5a005fff 100644 --- a/cmdline/memory.c +++ b/cmdline/memory.c @@ -157,3 +157,13 @@ char* strdup_nofail(const char* str) return ptr; } +char* strndup_nofail(const char* str, size_t size) +{ + char* ptr = malloc_nofail(size + 1); + + memcpy(ptr, str, size); + ptr[size] = 0; + + return ptr; +} + diff --git a/cmdline/memory.h b/cmdline/memory.h index d2ed0b9b..76950e67 100644 --- a/cmdline/memory.h +++ b/cmdline/memory.h @@ -36,6 +36,12 @@ void* calloc_nofail(size_t count, size_t size); */ char* strdup_nofail(const char* str); +/** + * Safe strndup with known length. + * If no memory is available, it aborts. + */ +char* strndup_nofail(const char* str, size_t size); + /** * Helper for printing an error about a failed allocation. */ diff --git a/cmdline/state.c b/cmdline/state.c index a233b628..613ea606 100644 --- a/cmdline/state.c +++ b/cmdline/state.c @@ -2596,6 +2596,8 @@ static void state_read_content(struct snapraid_state* state, const char* path, S /* LCOV_EXCL_STOP */ } + size_t sub_len; + ret = sgetbs(f, sub, sizeof(sub)); if (ret < 0) { /* LCOV_EXCL_START */ @@ -2604,6 +2606,8 @@ static void state_read_content(struct snapraid_state* state, const char* path, S os_abort(); /* LCOV_EXCL_STOP */ } + sub_len = ret; + if (!path_is_sub(sub)) { /* LCOV_EXCL_START */ decoding_error(path, f); @@ -2616,12 +2620,12 @@ static void state_read_content(struct snapraid_state* state, const char* path, S * Allocate without initializing blocks. The runs below cover the whole * file and set every state and hash before exposing the loaded state. */ - file = file_alloc(state->block_size, sub, v_size, v_mtime_sec, v_mtime_nsec, v_inode); + file = file_alloc_len(state->block_size, sub, sub_len, v_size, v_mtime_sec, v_mtime_nsec, v_inode); /* insert the file in the file containers */ if (file->inode != INODE_INVALID) tommy_hashdyn_insert(&disk->inodeset, &file->nodeset, file, file_inode_hash(file->inode)); - tommy_hashdyn_insert(&disk->pathset, &file->pathset, file, file_path_hash(file->sub)); + tommy_hashdyn_insert(&disk->pathset, &file->pathset, file, file_path_hash_len(file->sub, sub_len)); tommy_hashdyn_insert(&disk->stampset, &file->stampset, file, file_stamp_hash(file->size, file->mtime_sec, file->mtime_nsec)); tommy_list_insert_tail(&disk->filelist, &file->nodelist, file); @@ -3015,6 +3019,8 @@ static void state_read_content(struct snapraid_state* state, const char* path, S uint64_t v_mtime_sec; uint32_t v_mtime_nsec; + size_t sub_len; + ret = sgetbs(f, sub, sizeof(sub)); if (ret < 0) { /* LCOV_EXCL_START */ @@ -3023,6 +3029,8 @@ static void state_read_content(struct snapraid_state* state, const char* path, S os_abort(); /* LCOV_EXCL_STOP */ } + sub_len = ret; + if (!path_is_sub(sub)) { /* LCOV_EXCL_START */ decoding_error(path, f); @@ -3079,7 +3087,7 @@ static void state_read_content(struct snapraid_state* state, const char* path, S --v_mtime_nsec; /* allocate the file */ - struct snapraid_dealloc* dealloc = dealloc_alloc(state->block_size, sub, v_size, v_mtime_sec, v_mtime_nsec); + struct snapraid_dealloc* dealloc = dealloc_alloc_len(state->block_size, sub, sub_len, v_size, v_mtime_sec, v_mtime_nsec); log_tag("content_info:dealloc_entry:%s:%s:%" PRIu64 ":%" PRIu64 ":%u\n", disk->name, esc_tag(dealloc->sub), dealloc->size, dealloc->mtime_sec, dealloc->mtime_nsec); @@ -3124,6 +3132,8 @@ static void state_read_content(struct snapraid_state* state, const char* path, S /* symlink */ char sub[PATH_MAX]; char linkto[PATH_MAX]; + size_t sub_len; + size_t linkto_len; struct snapraid_link* slink; struct snapraid_disk* disk; uint32_t mapping; @@ -3146,6 +3156,7 @@ static void state_read_content(struct snapraid_state* state, const char* path, S os_abort(); /* LCOV_EXCL_STOP */ } + sub_len = ret; if (!path_is_sub(sub)) { /* LCOV_EXCL_START */ @@ -3163,12 +3174,13 @@ static void state_read_content(struct snapraid_state* state, const char* path, S os_abort(); /* LCOV_EXCL_STOP */ } + linkto_len = ret; /* allocate the link as symbolic link */ - slink = link_alloc(sub, linkto, FILE_IS_SYMLINK); + slink = link_alloc_len(sub, sub_len, linkto, linkto_len, FILE_IS_SYMLINK); /* insert the link in the link containers */ - tommy_hashdyn_insert(&disk->linkset, &slink->nodeset, slink, link_name_hash(slink->sub)); + tommy_hashdyn_insert(&disk->linkset, &slink->nodeset, slink, link_name_hash_len(slink->sub, sub_len)); tommy_list_insert_tail(&disk->linklist, &slink->nodelist, slink); /* stat */ @@ -3177,6 +3189,8 @@ static void state_read_content(struct snapraid_state* state, const char* path, S /* hardlink */ char sub[PATH_MAX]; char linkto[PATH_MAX]; + size_t sub_len; + size_t linkto_len; struct snapraid_link* slink; struct snapraid_disk* disk; uint32_t mapping; @@ -3199,6 +3213,7 @@ static void state_read_content(struct snapraid_state* state, const char* path, S os_abort(); /* LCOV_EXCL_STOP */ } + sub_len = ret; if (!path_is_sub(sub)) { /* LCOV_EXCL_START */ @@ -3216,6 +3231,7 @@ static void state_read_content(struct snapraid_state* state, const char* path, S os_abort(); /* LCOV_EXCL_STOP */ } + linkto_len = ret; if (!path_is_sub(linkto)) { /* LCOV_EXCL_START */ @@ -3226,10 +3242,10 @@ static void state_read_content(struct snapraid_state* state, const char* path, S } /* allocate the link as hard link */ - slink = link_alloc(sub, linkto, FILE_IS_HARDLINK); + slink = link_alloc_len(sub, sub_len, linkto, linkto_len, FILE_IS_HARDLINK); /* insert the link in the link containers */ - tommy_hashdyn_insert(&disk->linkset, &slink->nodeset, slink, link_name_hash(slink->sub)); + tommy_hashdyn_insert(&disk->linkset, &slink->nodeset, slink, link_name_hash_len(slink->sub, sub_len)); tommy_list_insert_tail(&disk->linklist, &slink->nodelist, slink); /* stat */ @@ -3237,6 +3253,7 @@ static void state_read_content(struct snapraid_state* state, const char* path, S } else if (c == 'r') { /* dir */ char sub[PATH_MAX]; + size_t sub_len; struct snapraid_dir* dir; struct snapraid_disk* disk; uint32_t mapping; @@ -3259,6 +3276,7 @@ static void state_read_content(struct snapraid_state* state, const char* path, S os_abort(); /* LCOV_EXCL_STOP */ } + sub_len = ret; if (!path_is_sub(sub)) { /* LCOV_EXCL_START */ @@ -3269,10 +3287,10 @@ static void state_read_content(struct snapraid_state* state, const char* path, S } /* allocate the dir */ - dir = dir_alloc(sub); + dir = dir_alloc_len(sub, sub_len); /* insert the dir in the dir containers */ - tommy_hashdyn_insert(&disk->dirset, &dir->nodeset, dir, dir_name_hash(dir->sub)); + tommy_hashdyn_insert(&disk->dirset, &dir->nodeset, dir, dir_name_hash_len(dir->sub, sub_len)); tommy_list_insert_tail(&disk->dirlist, &dir->nodelist, dir); /* stat */