From 7d4e774cf74e234bad99278870cf1d52fa1939fb Mon Sep 17 00:00:00 2001 From: apio Date: Sun, 25 Dec 2022 14:33:47 +0100 Subject: [PATCH] Rewrite the entire API just to eliminate heap allocations Just bumping minor because nobody uses this and I don't want to jump up to 2.0.0 --- CMakeLists.txt | 2 +- README.md | 51 ++++++++++--------------- minitar.h | 15 +++----- src/tar.c | 102 ++++++++++++++++++++----------------------------- src/util.c | 10 ----- 5 files changed, 68 insertions(+), 112 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ba60d21..fadfa44 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,6 +1,6 @@ cmake_minimum_required(VERSION 3.8..3.22) -project(minitar LANGUAGES C VERSION 1.2.1) +project(minitar LANGUAGES C VERSION 1.3.0) set(SOURCES src/tar.c diff --git a/README.md b/README.md index 9310a3d..c93e035 100644 --- a/README.md +++ b/README.md @@ -21,21 +21,19 @@ int main(int argc, char** argv) fprintf(stderr, "Usage: %s [file]\n", argv[0]); return 1; } - struct minitar* mp = minitar_open(argv[1]); - if(!mp) + struct minitar mp; + if(minitar_open(argv[1], &mp) != 0) { perror(argv[1]); return 1; } - struct minitar_entry* entry; + struct minitar_entry entry; do { - entry = minitar_read_entry(mp); - if(entry) { - printf("%s\n", entry->metadata.path); - minitar_free_entry(entry); - } - } while(entry); - minitar_close(mp); + if(minitar_read_entry(&mp, &entry) == 0) { + printf("%s\n", entry.metadata.path); + } else break; + } while(true); + minitar_close(&mp); } ``` @@ -47,27 +45,20 @@ The user-facing API (functions defined in `minitar.h` and documented in this REA ## Functions ### minitar_open -`struct minitar* minitar_open(const char* pathname)` +`int minitar_open(const char* pathname, struct minitar* mp)` -Opens a tar archive for reading, and returns a heap-allocated `struct minitar` which must be freed with `minitar_close()` after using it. If opening the file or allocating the struct fails, returns NULL. - -A `struct minitar` is opaque, and should only be passed to other minitar functions. You should not care about its contents. +Initializes the caller-provided `mp` structure by opening the archive pointed to by `pathname` for reading. Returns 0 on success, anything else is failure. ### minitar_read_entry -`struct minitar_entry* minitar_read_entry(struct minitar* mp)` +`int minitar_read_entry(struct minitar* mp, struct minitar_entry* out)` -Reads the next entry from a `struct minitar` which should be the return value of a previous call to `minitar_open()`. The return value is a heap-allocated `struct minitar_entry`, which should be freed with `minitar_free_entry()` when no longer needed. +Reads the next entry from a `struct minitar` which should be initialized by a previous call to `minitar_open()` and stores the result in `out`. -This structure consists of the file metadata (in the `metadata` field), and other internally-used values. +The `minitar_entry` structure consists of the file metadata (in the `metadata` field), and other internally-used values. To read the contents of an entry, you should allocate a buffer large enough to hold `metadata.size` bytes and pass it to `minitar_read_contents()`. -This function returns NULL on end-of-file (when all entries have been read). - -### minitar_free_entry -`void minitar_free_entry(struct minitar_entry* entry)` - -Frees the heap-allocated `struct minitar_entry`. The pointer passed to `minitar_free_entry()` should be the return value of a previous call to `minitar_read_entry()`, `minitar_find_by_name()`, `minitar_find_by_path()` or `minitar_find_any_of()`. +This function returns 0 on success and -1 on end-of-file (when all entries have been read). ### minitar_rewind `void minitar_rewind(struct minitar* mp)` @@ -75,26 +66,26 @@ Frees the heap-allocated `struct minitar_entry`. The pointer passed to `minitar_ Rewinds the `struct minitar` back to the beginning of the archive file, which means that the next call to `minitar_read_entry()` will return the first entry instead of the entry after the last read entry. ### minitar_find_by_name -`struct minitar_entry* minitar_find_by_name(struct minitar* mp, const char* name)` +`int minitar_find_by_name(struct minitar* mp, const char* name, struct minitar_entry* out)` -Returns the first entry with a matching name, or NULL if none are found. The return value is a `struct minitar_entry`, which is heap-allocated and should be freed after use with `minitar_free_entry()`. This structure is already documented in the entry documenting `minitar_read_entry()`. +Stores the first entry with a matching name in `out` and returns 0, or non-zero if none are found. In this case, the state of `out` is unspecified and might have been changed by the function. This function starts searching from the current archive position, which means that to find a matching entry in the entire archive `minitar_rewind()` should be called on it first. -The state of `mp` after `minitar_find_by_name()` returns is unspecified, but a successive call to `minitar_find_by_name()` will return the next matching entry, if there is one. (Calling `minitar_find_by_name()` in a loop until it returns NULL will return all matching entries.) +The state of `mp` after `minitar_find_by_name()` returns is unspecified, but a successive call to `minitar_find_by_name()` will find the next matching entry, if there is one. (Calling `minitar_find_by_name()` in a loop until it returns non-zero will return all matching entries.) In order to perform other minitar operations on the archive, `minitar_rewind()` should probably be called first, to get a known state. ### minitar_find_by_path -`struct minitar_entry* minitar_find_by_path(struct minitar* mp, const char* path)` +`int minitar_find_by_path(struct minitar* mp, const char* path, struct minitar_entry* out)` Same as `minitar_find_by_name()`, but matches the full path inside the archive instead of the file name. ### minitar_find_any_of -`struct minitar_entry* minitar_find_any_of(struct minitar* mp, enum minitar_file_type type)` +`int minitar_find_any_of(struct minitar* mp, enum minitar_file_type type, struct minitar_entry* out)` -Same as `minitar_find_by_name()`, but matches the file type instead of the name. As with `minitar_find_by_name()`, this function starts searching from the current archive position and calling it in a loop until it returns NULL will return all matching entries. +Same as `minitar_find_by_name()`, but matches the file type instead of the name. As with `minitar_find_by_name()`, this function starts searching from the current archive position and calling it in a loop until it returns -1 will find all matching entries. ### minitar_read_contents `size_t minitar_read_contents(struct minitar* mp, struct minitar_entry* entry, char* buf, size_t max)` @@ -112,7 +103,7 @@ The contents are not null-terminated. If you want null-termination (keep in mind ### minitar_close `int minitar_close(struct minitar* mp)` -Closes the tar archive file `mp` points to and frees the heap memory it was using. The pointer passed to `minitar_close()` should be the return value of a previous call to `minitar_open()`. +Closes the tar archive file `mp` points to. The pointer passed to `minitar_close()` should be the return value of a previous call to `minitar_open()`. Returns 0 on success, everything else is failure and you should check `errno`. diff --git a/minitar.h b/minitar.h index 43cbe22..77036d2 100644 --- a/minitar.h +++ b/minitar.h @@ -3,14 +3,10 @@ #include #include -#ifdef _IN_MINITAR struct minitar { FILE* stream; }; -#else -struct minitar; -#endif enum minitar_file_type { @@ -43,13 +39,12 @@ extern "C" { #endif - struct minitar* minitar_open(const char* pathname); - struct minitar_entry* minitar_read_entry(struct minitar* mp); - void minitar_free_entry(struct minitar_entry* entry); + int minitar_open(const char* pathname, struct minitar* out); + int minitar_read_entry(struct minitar* mp, struct minitar_entry* out); void minitar_rewind(struct minitar* mp); - struct minitar_entry* minitar_find_by_name(struct minitar* mp, const char* name); - struct minitar_entry* minitar_find_by_path(struct minitar* mp, const char* path); - struct minitar_entry* minitar_find_any_of(struct minitar* mp, enum minitar_file_type type); + int minitar_find_by_name(struct minitar* mp, const char* name, struct minitar_entry* out); + int minitar_find_by_path(struct minitar* mp, const char* path, struct minitar_entry* out); + int minitar_find_any_of(struct minitar* mp, enum minitar_file_type type, struct minitar_entry* out); size_t minitar_read_contents(struct minitar* mp, struct minitar_entry* entry, char* buf, size_t max); int minitar_close(struct minitar* mp); diff --git a/src/tar.c b/src/tar.c index 2eeb068..0b83920 100644 --- a/src/tar.c +++ b/src/tar.c @@ -1,4 +1,3 @@ -#define _IN_MINITAR #include "tar.h" #include "minitar.h" #include @@ -8,73 +7,62 @@ int minitar_read_header(struct minitar*, struct tar_header*); int minitar_validate_header(const struct tar_header*); void minitar_parse_metadata_from_tar_header(const struct tar_header*, struct minitar_entry_metadata*); -struct minitar_entry* minitar_dup_entry(const struct minitar_entry*); size_t minitar_align_up_to_block_size(size_t); -struct minitar* minitar_open(const char* pathname) +int minitar_open(const char* pathname, struct minitar* out) { FILE* fp = fopen(pathname, "rb"); // On some systems, opening the file in binary mode might be necessary to read the file properly. - if (!fp) return NULL; - struct minitar* mp = malloc(sizeof(struct minitar)); - if (!mp) - { - fclose(fp); - return NULL; - } - mp->stream = fp; - return mp; + if (!fp) return -1; + out->stream = fp; + return 0; } int minitar_close(struct minitar* mp) { - int rc = fclose(mp->stream); - free(mp); - return rc; + return fclose(mp->stream); } // Try to read a valid header, and construct an entry from it. If the 512-byte block at the current read offset is not a // valid header, valid is set to 0 so we can try again with the next block. In any other case, valid is set to 1. This // helps distinguish valid return values, null pointers that should be returned to the user (for example, EOF), and // invalid headers where we should just try again until we find a valid one. -static struct minitar_entry* minitar_try_to_read_valid_entry(struct minitar* mp, int* valid) +static int minitar_try_to_read_valid_entry(struct minitar* mp, struct minitar_entry* out, int* valid) { - struct minitar_entry entry; struct tar_header hdr; *valid = 1; - if (!minitar_read_header(mp, &hdr)) return NULL; + if (!minitar_read_header(mp, &hdr)) return -1; if (!minitar_validate_header(&hdr)) { *valid = 0; - return NULL; + return -1; } // Fetch the current read position (which is currently pointing to the start of the entry's contents), so we can // return back to it when reading the contents of this entry using minitar_read_contents(). - if (fgetpos(mp->stream, &entry.position)) return NULL; + if (fgetpos(mp->stream, &out->position)) return -1; - minitar_parse_metadata_from_tar_header(&hdr, &entry.metadata); - if (entry.metadata.size) + minitar_parse_metadata_from_tar_header(&hdr, &out->metadata); + if (out->metadata.size) { - size_t size_in_archive = minitar_align_up_to_block_size(entry.metadata.size); + size_t size_in_archive = minitar_align_up_to_block_size(out->metadata.size); if (fseek(mp->stream, size_in_archive, SEEK_CUR)) // move over to the next block, skipping over the file contents { - return NULL; + return -1; } } - return minitar_dup_entry(&entry); + return 0; } -struct minitar_entry* minitar_read_entry(struct minitar* mp) +int minitar_read_entry(struct minitar* mp, struct minitar_entry* out) { - int valid; - struct minitar_entry* result; + int valid, result; do { - result = minitar_try_to_read_valid_entry(mp, &valid); + result = minitar_try_to_read_valid_entry(mp, out, &valid); } while (!valid); // Skip over invalid entries return result; } @@ -84,51 +72,43 @@ void minitar_rewind(struct minitar* mp) rewind(mp->stream); } -void minitar_free_entry(struct minitar_entry* entry) +int minitar_find_by_name(struct minitar* mp, const char* name, struct minitar_entry* out) { - free(entry); + int rc; + do { + rc = minitar_read_entry(mp, out); + if (rc == 0) + { + if (!strcmp(out->metadata.name, name)) return 0; + } + } while (rc == 0); + return -1; } -struct minitar_entry* minitar_find_by_name(struct minitar* mp, const char* name) +int minitar_find_by_path(struct minitar* mp, const char* path, struct minitar_entry* out) { - struct minitar_entry* entry; + int rc; do { - entry = minitar_read_entry(mp); - if (entry) + rc = minitar_read_entry(mp, out); + if (rc == 0) { - if (!strcmp(entry->metadata.name, name)) return entry; - minitar_free_entry(entry); + if (!strcmp(out->metadata.path, path)) return 0; } - } while (entry); - return NULL; + } while (rc == 0); + return -1; } -struct minitar_entry* minitar_find_by_path(struct minitar* mp, const char* path) +int minitar_find_any_of(struct minitar* mp, enum minitar_file_type type, struct minitar_entry* out) { - struct minitar_entry* entry; + int rc; do { - entry = minitar_read_entry(mp); - if (entry) + rc = minitar_read_entry(mp, out); + if (rc == 0) { - if (!strcmp(entry->metadata.path, path)) return entry; - minitar_free_entry(entry); + if (out->metadata.type == type) return 0; } - } while (entry); - return NULL; -} - -struct minitar_entry* minitar_find_any_of(struct minitar* mp, enum minitar_file_type type) -{ - struct minitar_entry* entry; - do { - entry = minitar_read_entry(mp); - if (entry) - { - if (entry->metadata.type == type) return entry; - minitar_free_entry(entry); - } - } while (entry); - return NULL; + } while (rc == 0); + return -1; } size_t minitar_read_contents(struct minitar* mp, struct minitar_entry* entry, char* buf, size_t max) diff --git a/src/util.c b/src/util.c index 15be7c5..1978fc0 100644 --- a/src/util.c +++ b/src/util.c @@ -1,4 +1,3 @@ -#define _IN_MINITAR #include "minitar.h" #include "tar.h" #include @@ -182,13 +181,4 @@ int minitar_read_header(struct minitar* mp, struct tar_header* hdr) if (rc == 0 && ferror(mp->stream)) minitar_panic("Error while reading file header from tar archive"); if (rc < sizeof *hdr) minitar_panic("Valid tar files should be split in 512-byte blocks"); return 1; -} - -// Create a heap-allocated copy of an entry on the stack. -struct minitar_entry* minitar_dup_entry(const struct minitar_entry* original) -{ - struct minitar_entry* new = malloc(sizeof *original); - if (!new) return NULL; - memcpy(new, original, sizeof *new); - return new; } \ No newline at end of file