vieter-v/libvieter
Archived
2
0
Fork 2
Code Issues 2 Pull requests 2 Releases Activity

Refactor package module into C #6

Open
GreekStapler wants to merge 25 commits from GreekStapler/libvieter:package into dev
pull from: GreekStapler/libvieter:package
merge into: vieter-v:dev
Conversation 1 Commits 25 Files changed 17 +107 -107
8 changed files with 107 additions and 107 deletions
Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit bcae64e023 - Show all commits

8
include/vieter_package.h
View file

@ -9,22 +9,22 @@
#include <archive.h>
#include <archive_entry.h>
typedef struct pkg Pkg;
typedef struct vieter_package vieter_package;
/*
* Parse package file into something usable by libvieter.
* The pointer returned by this function will need to freed at a later point.
*/
Pkg *vieter_package_read_archive(const char *pkg_path);
vieter_package *vieter_package_read_archive(const char *pkg_path);
/*
* Deallocate a package.
*/
void vieter_package_free(Pkg ** ptp);
void vieter_package_free(vieter_package ** ptp);
Jef Roosens commented 2023-01-29 13:08:06 +01:00
Outdated
Copy link

I don't personally use this style of free function right now, but it might be interesting to migrate all free functions to this style indeed. I'm assuming the pointer-to-pointer is so you can set the variable containing the pointer to NULL as well?

I don't personally use this style of free function right now, but it might be interesting to migrate all free functions to this style indeed. I'm assuming the pointer-to-pointer is so you can set the variable containing the pointer to NULL as well?
GreekStapler commented 2023-01-29 16:10:39 +01:00
Outdated
Copy link

Yeah, dangling pointers can cause some real headaches, so I prefer to have free functions set the pointer to NULL themselves to make it less error prone.

Yeah, dangling pointers can cause some real headaches, so I prefer to have free functions set the pointer to NULL themselves to make it less error prone.
/*
* Create string that will become the package's desc file.
*/
char *vieter_package_to_description(Pkg *pkg);
char *vieter_package_to_description(vieter_package *pkg);
#endif

46
src/package/vieter_package.c
View file

@ -3,23 +3,23 @@
#define SMALL_BUFF_SIZE 128
#define ADD_STRING(section, field) if (info->field != 0) { \
snprintf(aux, SMALL_BUFF_SIZE, section, info->field); \
#define ADD_STRING(section, field) if (pkg_info->field != 0) { \
snprintf(aux, SMALL_BUFF_SIZE, section, pkg_info->field); \
Jef Roosens commented 2023-01-29 13:30:13 +01:00
Outdated
Copy link

General note for snprintf usage: what happens if the value is larger than the buffer? Is it simply cut off? Cuz if so, we shouldn't be making assumptions about the length of the input.

General note for `snprintf` usage: what happens if the value is larger than the buffer? Is it simply cut off? Cuz if so, we shouldn't be making assumptions about the length of the input.
GreekStapler commented 2023-01-29 18:29:40 +01:00
Outdated
Copy link

I used snprintf because I didn't want to risk any buffer overflows, but it can truncate perfectly valid inputs if they are long enough (e.g. very long url). I'll turn aux into a malloc'd string and use the return value of snprintf to reallocate memory as is necessary without any overflows.

I used `snprintf` because I didn't want to risk any buffer overflows, but it can truncate perfectly valid inputs if they are long enough (e.g. very long url). I'll turn `aux` into a malloc'd string and use the return value of `snprintf` to reallocate memory as is necessary without any overflows.
if (buff_size < strlen(description) + SMALL_BUFF_SIZE + 1) { \
description = realloc(description, buff_size * 2); \
buff_size *= 2; \
} \
strcat(description, aux); \
}
#define ADD_ARRAY(section, field) i = 0; if (info->field != NULL) { \
snprintf(aux, SMALL_BUFF_SIZE, section, info->field->array[i]); i++; \
#define ADD_ARRAY(section, field) i = 0; if (pkg_info->field != NULL) { \
snprintf(aux, SMALL_BUFF_SIZE, section, pkg_info->field->array[i]); i++; \
if (buff_size < strlen(description) + SMALL_BUFF_SIZE + 1) { \
description = realloc(description, buff_size * 2); \
buff_size *= 2; \
} \
strcat(description, aux); \
while (info->field->array[i] != NULL) { \
snprintf(aux, SMALL_BUFF_SIZE, "\n%s", info->field->array[i]); i++; \
while (pkg_info->field->array[i] != NULL) { \
Jef Roosens commented 2023-01-29 13:34:10 +01:00
Outdated
Copy link

This check is not safe, the dynarray code doesn't make any guarantees about empty fields being NULL, they're simply malloc'ed and not initialised.

This check is not safe, the dynarray code doesn't make any guarantees about empty fields being NULL, they're simply malloc'ed and not initialised.
snprintf(aux, SMALL_BUFF_SIZE, "\n%s", pkg_info->field->array[i]); i++; \
if (buff_size < strlen(description) + SMALL_BUFF_SIZE + 1) { \
description = realloc(description, buff_size * 2); \
buff_size *= 2; \
@ -37,11 +37,11 @@ static char *ignored_names[5] = {
};
static size_t ignored_words_len = sizeof(ignored_names) / sizeof(char *);
Pkg *vieter_package_init() {
return calloc(sizeof(pkg_info), 1);
vieter_package *vieter_package_init() {
return calloc(sizeof(vieter_package_info), 1);
GreekStapler marked this conversation as resolved Outdated
Jef Roosens commented 2023-01-29 13:13:59 +01:00
Outdated
Copy link

Shouldn't this be sizeof(a package)?

Shouldn't this be `sizeof(a package)`?
}
Pkg *vieter_package_read_archive(const char *pkg_path) {
vieter_package *vieter_package_read_archive(const char *pkg_path) {
struct archive *a = archive_read_new();
struct archive_entry *entry = archive_entry_new();
@ -64,8 +64,8 @@ Pkg *vieter_package_read_archive(const char *pkg_path) {
int compression_code = archive_filter_code(a, 0);
const char *path_name;
pkg_info *info;
dynarray *files = vieter_package_dynarray_init(16);
vieter_package_info *pkg_info;
vieter_package_dynarray *files = vieter_package_dynarray_init(16);
vieter_package_dynarray_add(files, "%FILES%");
while (archive_read_next_header(a, &entry) == ARCHIVE_OK) {
@ -90,9 +90,9 @@ Pkg *vieter_package_read_archive(const char *pkg_path) {
char *buf = malloc(size);
archive_read_data(a, buf, size);
// Parse package info string into a struct
info = vieter_package_info_init();
vieter_package_info_parse(info, buf);
// Parse package vieter_package_info string into a struct
pkg_info = vieter_package_info_init();
vieter_package_info_parse(pkg_info, buf);
free(buf);
} else {
@ -107,21 +107,21 @@ Pkg *vieter_package_read_archive(const char *pkg_path) {
return NULL;
}
info->csize = stats.st_size;
pkg_info->csize = stats.st_size;
archive_read_free(a);
// Create final return value
Pkg *pkg = vieter_package_init();
vieter_package *pkg = vieter_package_init();
pkg->path = strdup(pkg_path);
pkg->info = info;
pkg->info = pkg_info;
pkg->files = files;
pkg->compression = compression_code;
return pkg;
}
void vieter_package_sha256sum(Pkg *pkg, char *res) {
void vieter_package_sha256sum(vieter_package *pkg, char *res) {
Jef Roosens commented 2023-01-29 13:19:36 +01:00
Outdated
Copy link

As mentioned on matrix, this function should become a streaming one.

As mentioned on matrix, this function should become a streaming one.
FILE *f = fopen(pkg->path, "r");
fseek(f, 0, SEEK_END);
size_t size = ftell(f);
@ -170,8 +170,8 @@ void vieter_package_sha256sum(Pkg *pkg, char *res) {
res[j] = '\0';
}
char *vieter_package_to_description(Pkg *pkg) {
pkg_info *info = pkg->info;
char *vieter_package_to_description(vieter_package *pkg) {
vieter_package_info *pkg_info = pkg->info;
size_t buff_size = 1024;
Jef Roosens commented 2023-01-29 13:21:52 +01:00
Outdated
Copy link

This should also use SMALL_BUFF_SIZE I think, just for consistency.

This should also use `SMALL_BUFF_SIZE` I think, just for consistency.
char aux[SMALL_BUFF_SIZE];
@ -181,8 +181,8 @@ char *vieter_package_to_description(Pkg *pkg) {
// special case for FILENAME
// assuming .pkg.tar.zst; other formats are valid, this should account for that
snprintf(aux, SMALL_BUFF_SIZE, "%%FILENAME%%\n%s-%s-%s.pkg.tar.zst", info->name, info->version,
info->arch);
snprintf(aux, SMALL_BUFF_SIZE, "%%FILENAME%%\n%s-%s-%s.pkg.tar.zst", pkg_info->name,
Jef Roosens commented 2023-01-29 13:23:41 +01:00
Outdated
Copy link

Vieter doesn't only supports zstd-compressed archives. Currently it also accepts xz- or gzip-compressed archives, so this should be accounted for.

This choice was made noteably because Archlinux ARM uses xz-compressed archives instead of zstd.

Vieter doesn't only supports zstd-compressed archives. Currently it also accepts xz- or gzip-compressed archives, so this should be accounted for. This choice was made noteably because Archlinux ARM uses xz-compressed archives instead of zstd.
pkg_info->version, pkg_info->arch);
strcpy(description, aux);
ADD_STRING("\n\n%%NAME%%\n%s", name);
@ -222,7 +222,7 @@ char *vieter_package_to_description(Pkg *pkg) {
return description;
}
void vieter_package_free(Pkg **ptp) {
void vieter_package_free(vieter_package **ptp) {
FREE_STRING((*ptp)->path);
vieter_package_info_free((*ptp)->info);
vieter_package_dynarray_free((*ptp)->files);

12
src/package/vieter_package_dynarray.c
View file

@ -1,15 +1,15 @@
#include "vieter_package_dynarray.h"
dynarray *vieter_package_dynarray_init(size_t initial_capacity) {
dynarray *da = malloc(sizeof(dynarray));
vieter_package_dynarray *vieter_package_dynarray_init(size_t initial_capacity) {
vieter_package_dynarray *da = malloc(sizeof(vieter_package_dynarray));
da->size = 0;
da->capacity = initial_capacity;
return da;
}
void vieter_package_dynarray_add(dynarray *da, const char *s) {
// An empty dynarray does not have an allocated internal array yet
void vieter_package_dynarray_add(vieter_package_dynarray *da, const char *s) {
// An empty vieter_package_dynarray does not have an allocated internal array yet
if (da->size == 0) {
da->array = malloc(sizeof(char*) * da->capacity);
}
@ -24,7 +24,7 @@ void vieter_package_dynarray_add(dynarray *da, const char *s) {
da->size++;
}
void vieter_package_dynarray_free(dynarray *da) {
void vieter_package_dynarray_free(vieter_package_dynarray *da) {
if (da == NULL) {
return;
}
@ -40,7 +40,7 @@ void vieter_package_dynarray_free(dynarray *da) {
free(da);
}
char **vieter_package_dynarray_convert(dynarray *da) {
char **vieter_package_dynarray_convert(vieter_package_dynarray *da) {
char **array = da->array;
da->array = NULL;

14
src/package/vieter_package_dynarray.h
View file

@ -5,8 +5,8 @@
#include <string.h>
#include "vieter_package.h"
typedef struct dynarray dynarray;
struct dynarray {
typedef struct vieter_package_dynarray vieter_package_dynarray;
struct vieter_package_dynarray {
char **array;
size_t capacity;
size_t size;
@ -15,22 +15,22 @@ struct dynarray {
/*
* Allocate a dynamic array.
*/
dynarray *vieter_package_dynarray_init(size_t initial_capacity);
vieter_package_dynarray *vieter_package_dynarray_init(size_t initial_capacity);
/*
* Initialise array (if it's not already initialised) and insert a string.
*/
void vieter_package_dynarray_add(dynarray *da, const char * s);
void vieter_package_dynarray_add(vieter_package_dynarray *da, const char * s);
/*
* Deallocate dynamic array.
*/
void vieter_package_dynarray_free(dynarray *da);
void vieter_package_dynarray_free(vieter_package_dynarray *da);
/*
* Convert a dynarray into an array by freeing all its surrounding components
* Convert a vieter_package_dynarray into an array by freeing all its surrounding components
* and returning the underlying array pointer.
*/
char **vieter_package_dynarray_convert(dynarray *da);
char **vieter_package_dynarray_convert(vieter_package_dynarray *da);
#endif

52
src/package/vieter_package_info.c
View file

@ -6,14 +6,14 @@
value_ptr += strlen(key_ptr);\
Jef Roosens commented 2023-01-29 13:39:30 +01:00
Outdated
Copy link

How does this work? strstr doens't write any terminating NULL bytes for the substring or anything, so won't this strlen call return the entire remaining size of the pkginfo string?

How does this work? `strstr` doens't write any terminating NULL bytes for the substring or anything, so won't this `strlen` call return the entire remaining size of the pkginfo string?
GreekStapler commented 2023-02-02 11:56:25 +01:00
Outdated
Copy link

The strlen is called on key_ptr which is just a small self contained string (e.g. "\ngroup = ") that is null terminated.

The pkginfo string in here is value_ptr.

The `strlen` is called on `key_ptr` which is just a small self contained string (e.g. "\ngroup = ") that is null terminated. The pkginfo string in here is `value_ptr`.
tail_ptr = strchr(value_ptr, '\n');\
tail_ptr[0] = '\0'; \
info->field = strdup(value_ptr); \
pkg_info->field = strdup(value_ptr); \
tail_ptr[0] = '\n'; \
} value_ptr = tail_ptr;
#define PKG_INFO_INT(key_ptr, field) value_ptr = strstr(value_ptr, key_ptr) + strlen(key_ptr);\
tail_ptr = strchr(value_ptr, '\n');\
tail_ptr[0] = '\0'; \
info->field = atoi(value_ptr); \
pkg_info->field = atoi(value_ptr); \
tail_ptr[0] = '\n'; \
value_ptr = tail_ptr;
@ -21,40 +21,40 @@
value_ptr = value_ptr + strlen(key_ptr);\
tail_ptr = strchr(value_ptr, '\n'); \
tail_ptr[0] = '\0'; \
if(info->field == NULL) { info->field = vieter_package_dynarray_init(4); } \
vieter_package_dynarray_add(info->field, value_ptr); \
if(pkg_info->field == NULL) { pkg_info->field = vieter_package_dynarray_init(4); } \
vieter_package_dynarray_add(pkg_info->field, value_ptr); \
tail_ptr[0] = '\n'; \
value_ptr = tail_ptr;\
} value_ptr = tail_ptr;
pkg_info *vieter_package_info_init() {
return calloc(1, sizeof(pkg_info));
vieter_package_info *vieter_package_info_init() {
return calloc(1, sizeof(vieter_package_info));
}
void vieter_package_info_free(pkg_info *info) {
FREE_STRING(info->name);
FREE_STRING(info->base);
FREE_STRING(info->version);
FREE_STRING(info->description);
FREE_STRING(info->url);
FREE_STRING(info->arch);
FREE_STRING(info->packager);
FREE_STRING(info->pgpsig);
void vieter_package_info_free(vieter_package_info *pkg_info) {
FREE_STRING(pkg_info->name);
FREE_STRING(pkg_info->base);
FREE_STRING(pkg_info->version);
FREE_STRING(pkg_info->description);
FREE_STRING(pkg_info->url);
FREE_STRING(pkg_info->arch);
FREE_STRING(pkg_info->packager);
FREE_STRING(pkg_info->pgpsig);
vieter_package_dynarray_free(info->groups);
vieter_package_dynarray_free(info->licenses);
vieter_package_dynarray_free(info->replaces);
vieter_package_dynarray_free(info->depends);
vieter_package_dynarray_free(info->conflicts);
vieter_package_dynarray_free(info->provides);
vieter_package_dynarray_free(info->optdepends);
vieter_package_dynarray_free(info->makedepends);
vieter_package_dynarray_free(info->checkdepends);
vieter_package_dynarray_free(pkg_info->groups);
vieter_package_dynarray_free(pkg_info->licenses);
vieter_package_dynarray_free(pkg_info->replaces);
vieter_package_dynarray_free(pkg_info->depends);
vieter_package_dynarray_free(pkg_info->conflicts);
vieter_package_dynarray_free(pkg_info->provides);
vieter_package_dynarray_free(pkg_info->optdepends);
vieter_package_dynarray_free(pkg_info->makedepends);
vieter_package_dynarray_free(pkg_info->checkdepends);
free(info);
free(pkg_info);
}
void vieter_package_info_parse(pkg_info *info, char *pkg_info_str) {
void vieter_package_info_parse(vieter_package_info *pkg_info, char *pkg_info_str) {
char *value_ptr = pkg_info_str, *tail_ptr;
PKG_INFO_STRING("\npkgname = ", name);

28
src/package/vieter_package_info.h
View file

@ -9,7 +9,7 @@
#include "vieter_package_dynarray.h"
typedef struct pkg_info {
typedef struct vieter_package_info {
char *name;
char *base;
char *version;
@ -23,30 +23,30 @@ typedef struct pkg_info {
char *pgpsig;
int64_t pgpsigsize;
dynarray *groups;
dynarray *licenses;
dynarray *replaces;
dynarray *depends;
dynarray *conflicts;
dynarray *provides;
dynarray *optdepends;
dynarray *makedepends;
dynarray *checkdepends;
} pkg_info;
vieter_package_dynarray *groups;
vieter_package_dynarray *licenses;
vieter_package_dynarray *replaces;
vieter_package_dynarray *depends;
vieter_package_dynarray *conflicts;
vieter_package_dynarray *provides;
vieter_package_dynarray *optdepends;
vieter_package_dynarray *makedepends;
vieter_package_dynarray *checkdepends;
} vieter_package_info;
/*
* Allocate and initialise a pkg_info pointer to hold .PKGINFO.
*/
pkg_info *vieter_package_info_init();
vieter_package_info *vieter_package_info_init();
/*
* Parse .PKGINFO file into something usable by libvieter.
*/
void vieter_package_info_parse(pkg_info *info, char *pkg_info_str);
void vieter_package_info_parse(vieter_package_info *pkg_info, char *pkg_info_str);
/*
* Deallocate a pkg_info pointer.
*/
void vieter_package_info_free(pkg_info *info);
void vieter_package_info_free(vieter_package_info *pkg_info);
#endif

6
src/package/vieter_package_internal.h
View file

@ -2,9 +2,9 @@
#include "vieter_package_info.h"
#include "vieter_package_dynarray.h"
struct pkg {
struct vieter_package {
char *path;
pkg_info *info;
dynarray *files;
vieter_package_info *info;
vieter_package_dynarray *files;
int compression;
};

48
test/package/test_package.c
View file

@ -12,34 +12,34 @@ void test_info_parse() {
fread(pkg_info_str, 1, size, f);
fclose(f);
pkg_info *info = vieter_package_info_init();
vieter_package_info_parse(info, pkg_info_str);
vieter_package_info *pkg_info = vieter_package_info_init();
vieter_package_info_parse(pkg_info, pkg_info_str);
TEST_CHECK(!strcmp(info->name, "xcursor-dmz"));
TEST_CHECK(!strcmp(info->base, "xcursor-dmz"));
TEST_CHECK(!strcmp(info->version, "0.4.5-2"));
TEST_CHECK(!strcmp(info->description, "Style neutral, scalable cursor theme"));
TEST_CHECK(!strcmp(info->url, "https://packages.debian.org/sid/dmz-cursor-theme"));
TEST_CHECK(info->build_date == 1673751613);
TEST_CHECK(!strcmp(info->packager, "Unknown Packager"));
TEST_CHECK(info->size == 3469584);
TEST_CHECK(!strcmp(info->arch, "any"));
TEST_CHECK(!strcmp(pkg_info->name, "xcursor-dmz"));
TEST_CHECK(!strcmp(pkg_info->base, "xcursor-dmz"));
TEST_CHECK(!strcmp(pkg_info->version, "0.4.5-2"));
TEST_CHECK(!strcmp(pkg_info->description, "Style neutral, scalable cursor theme"));
TEST_CHECK(!strcmp(pkg_info->url, "https://packages.debian.org/sid/dmz-cursor-theme"));
TEST_CHECK(pkg_info->build_date == 1673751613);
TEST_CHECK(!strcmp(pkg_info->packager, "Unknown Packager"));
TEST_CHECK(pkg_info->size == 3469584);
TEST_CHECK(!strcmp(pkg_info->arch, "any"));
TEST_CHECK(!strcmp(info->licenses->array[0], "MIT"));
TEST_CHECK(!strcmp(info->replaces->array[0], "test1"));
TEST_CHECK(!strcmp(info->groups->array[0], "x11"));
TEST_CHECK(!strcmp(info->conflicts->array[0], "test2"));
TEST_CHECK(!strcmp(info->conflicts->array[1], "test3"));
TEST_CHECK(!strcmp(info->provides->array[0], "test4"));
TEST_CHECK(!strcmp(info->depends->array[0], "test5"));
TEST_CHECK(!strcmp(info->depends->array[1], "test6"));
TEST_CHECK(!strcmp(info->optdepends->array[0], "test7"));
TEST_CHECK(!strcmp(info->makedepends->array[0], "xorg-xcursorgen"));
TEST_CHECK(!strcmp(info->checkdepends->array[0], "test8"));
TEST_CHECK(!strcmp(pkg_info->licenses->array[0], "MIT"));
TEST_CHECK(!strcmp(pkg_info->replaces->array[0], "test1"));
TEST_CHECK(!strcmp(pkg_info->groups->array[0], "x11"));
TEST_CHECK(!strcmp(pkg_info->conflicts->array[0], "test2"));
TEST_CHECK(!strcmp(pkg_info->conflicts->array[1], "test3"));
TEST_CHECK(!strcmp(pkg_info->provides->array[0], "test4"));
TEST_CHECK(!strcmp(pkg_info->depends->array[0], "test5"));
TEST_CHECK(!strcmp(pkg_info->depends->array[1], "test6"));
TEST_CHECK(!strcmp(pkg_info->optdepends->array[0], "test7"));
TEST_CHECK(!strcmp(pkg_info->makedepends->array[0], "xorg-xcursorgen"));
TEST_CHECK(!strcmp(pkg_info->checkdepends->array[0], "test8"));
}
void test_pkg_read_archive_files() {
Pkg *pkg = vieter_package_read_archive("./test/package/xcursor-dmz-0.4.5-2-any.pkg.tar.zst");
vieter_package *pkg = vieter_package_read_archive("./test/package/xcursor-dmz-0.4.5-2-any.pkg.tar.zst");
TEST_ASSERT_(pkg != NULL, "failure parsing pkg archive");
FILE *f = fopen("./test/package/files", "r");
@ -62,7 +62,7 @@ void test_pkg_read_archive_files() {
}
void test_pkg_read_archive_desc() {
Pkg *pkg = vieter_package_read_archive("./test/package/xcursor-dmz-0.4.5-2-any.pkg.tar.zst");
vieter_package *pkg = vieter_package_read_archive("./test/package/xcursor-dmz-0.4.5-2-any.pkg.tar.zst");
TEST_ASSERT_(pkg != NULL, "failure parsing pkg archive");
char *description = vieter_package_to_description(pkg);