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 +302 -291
4 changed files with 302 additions and 291 deletions
Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit e253e039c3 - Show all commits

5
include/vieter_package.h
View file

@ -17,12 +17,13 @@ vieter_package *vieter_package_init();
/*
* Parse package file into something usable by libvieter.
*/
vieter_package_error vieter_package_read_archive(vieter_package *pkg, const char *pkg_path);
vieter_package_error vieter_package_read_archive(vieter_package *pkg,
const char *pkg_path);
/*
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.
* Deallocate a package.
*/
void vieter_package_free(vieter_package ** ptp);
void vieter_package_free(vieter_package **ptp);
/*
* Create string that will become the package's desc file.

65
src/package/vieter_package.c
View file

@ -1,13 +1,15 @@
#include <stdlib.h>
#include <stdbool.h>
#include <archive.h>
#include <archive_entry.h>
#include <stdbool.h>
#include <stdlib.h>
#include "vieter_package_internal.h"
#include "sha256.h"
#include "vieter_package_internal.h"
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.
#define ADD_STRING(section, field) if (pkg_info->field != 0) { \
size_to_be_written = snprintf(aux, small_buff_size, section, pkg_info->field); \
#define ADD_STRING(section, field) \
if (pkg_info->field != 0) { \
size_to_be_written = \
snprintf(aux, small_buff_size, section, pkg_info->field); \
if (size_to_be_written > small_buff_size) { \
aux = realloc(aux, size_to_be_written + 1); \
small_buff_size = size_to_be_written + 1; \
@ -18,29 +20,29 @@
buff_size *= 2; \
} \
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.
strcat(description, aux); \
}
}
#define ADD_ARRAY(section, field) i = 0; if (pkg_info->field != NULL) { \
ADD_STRING(section, field->array[i]); i++; \
#define ADD_ARRAY(section, field) \
i = 0; \
if (pkg_info->field != NULL) { \
ADD_STRING(section, field->array[i]); \
i++; \
while (pkg_info->field->array[i] != NULL) { \
ADD_STRING("\n%s", field->array[i]); i++; \
ADD_STRING("\n%s", field->array[i]); \
i++; \
} \
}
}
static char *ignored_names[5] = {
".BUILDINFO",
".INSTALL",
".MTREE",
".PKGINFO",
".CHANGELOG"
};
static char *ignored_names[5] = {".BUILDINFO", ".INSTALL", ".MTREE", ".PKGINFO",
".CHANGELOG"};
static size_t ignored_words_len = sizeof(ignored_names) / sizeof(char *);
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)`?
}
vieter_package_error vieter_package_read_archive(vieter_package *pkg, const char *pkg_path) {
vieter_package_error vieter_package_read_archive(vieter_package *pkg,
const char *pkg_path) {
struct archive *a = archive_read_new();
struct archive_entry *entry = archive_entry_new();
@ -141,10 +143,10 @@ void vieter_package_sha256sum(vieter_package *pkg, char *res) {
free(in);
free(ctx);
// We need to convert the bytes in the hash to get a string representation of its hex values
// i.e. turn 1001 1111 into the string "9f"
// Each byte of the hash is going to turn into two bytes in the final string
// so we are going to convert each half byte into a char
// We need to convert the bytes in the hash to get a string representation of
// its hex values i.e. turn 1001 1111 into the string "9f" Each byte of the
// hash is going to turn into two bytes in the final string so we are going to
// convert each half byte into a char
unsigned int half_byte = 0;
int j = 0;
@ -154,9 +156,9 @@ void vieter_package_sha256sum(vieter_package *pkg, char *res) {
// each byte from becoming reversed in the final string
half_byte = hash[i] & 0b1111;
if (half_byte < 10) {
res[j+1] = half_byte + 48;
res[j + 1] = half_byte + 48;
} else {
res[j+1] = half_byte + 87;
res[j + 1] = half_byte + 87;
}
hash[i] = hash[i] >> 4;
half_byte = hash[i] & 0b1111;
@ -171,8 +173,6 @@ void vieter_package_sha256sum(vieter_package *pkg, char *res) {
res[j] = '\0';
}
char *vieter_package_to_description(vieter_package *pkg) {
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.
vieter_package_info *pkg_info = pkg->info;
@ -185,17 +185,19 @@ char *vieter_package_to_description(vieter_package *pkg) {
int i;
// special case for FILENAME
// assuming .pkg.tar.zst; other formats are valid, this should account for that
size_to_be_written = snprintf(aux, small_buff_size, "%%FILENAME%%\n%s-%s-%s.pkg.tar.zst", pkg_info->name,
pkg_info->version, pkg_info->arch);
// assuming .pkg.tar.zst; other formats are valid, this should account for
// that
size_to_be_written =
snprintf(aux, small_buff_size, "%%FILENAME%%\n%s-%s-%s.pkg.tar.zst",
pkg_info->name, pkg_info->version, pkg_info->arch);
// We neither want to let an arbritrarily long input to overflow the buffer
// nor to truncate perfectly valid inputs
if (size_to_be_written > small_buff_size) {
aux = realloc(aux, size_to_be_written + 1);
small_buff_size = size_to_be_written + 1;
snprintf(aux, small_buff_size, "%%FILENAME%%\n%s-%s-%s.pkg.tar.zst", pkg_info->name,
pkg_info->version, pkg_info->arch);
snprintf(aux, small_buff_size, "%%FILENAME%%\n%s-%s-%s.pkg.tar.zst",
pkg_info->name, pkg_info->version, pkg_info->arch);
}
strcpy(description, aux);
@ -241,5 +243,4 @@ void vieter_package_free(vieter_package **ptp) {
vieter_package_dynarray_free((*ptp)->files);
free(*ptp);
*ptp = NULL;
}

14
src/package/vieter_package_dynarray.c
View file

@ -9,21 +9,23 @@ vieter_package_dynarray *vieter_package_dynarray_init(size_t initial_capacity) {
}
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
// An empty vieter_package_dynarray does not have an allocated internal array
// yet
if (da->size == 0) {
da->array = malloc(sizeof(char*) * da->capacity);
da->array = malloc(sizeof(char *) * da->capacity);
// Initialise all char*'s to 0 so array[i] == NULL can be used to see if field is empty
memset(da->array, 0, sizeof(char*) * da->capacity);
// Initialise all char*'s to 0 so array[i] == NULL can be used to see if
// field is empty
memset(da->array, 0, sizeof(char *) * da->capacity);
}
// Double array size if it's full
else if (da->size == da->capacity) {
// if the realloc fails, access to memory in da->array is lost
da->array = realloc(da->array, sizeof(char*) * da->capacity * 2);
da->array = realloc(da->array, sizeof(char *) * da->capacity * 2);
da->capacity *= 2;
// Same as the previous memset, but only for newly allocated pointers
memset(da->array + da->size, 0, sizeof(char*) * da->capacity / 2);
memset(da->array + da->size, 0, sizeof(char *) * da->capacity / 2);
}
da->array[da->size] = strdup(s);

33
src/package/vieter_package_info.c
View file

@ -2,30 +2,37 @@
#include "vieter_package_info.h"
#define PKG_INFO_STRING(key_ptr, field) if ((value_ptr = strstr(value_ptr, key_ptr)) != NULL) { \
value_ptr += strlen(key_ptr);\
tail_ptr = strchr(value_ptr, '\n');\
#define PKG_INFO_STRING(key_ptr, field) \
if ((value_ptr = strstr(value_ptr, key_ptr)) != NULL) { \
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`.
value_ptr += strlen(key_ptr); \
tail_ptr = strchr(value_ptr, '\n'); \
tail_ptr[0] = '\0'; \
pkg_info->field = strdup(value_ptr); \
tail_ptr[0] = '\n'; \
} value_ptr = tail_ptr;
} \
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');\
#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'; \
pkg_info->field = atoi(value_ptr); \
tail_ptr[0] = '\n'; \
value_ptr = tail_ptr;
#define PKG_INFO_ARRAY(key_ptr, field) while((value_ptr = strstr(value_ptr, key_ptr)) != NULL){ \
value_ptr = value_ptr + strlen(key_ptr);\
#define PKG_INFO_ARRAY(key_ptr, field) \
while ((value_ptr = strstr(value_ptr, key_ptr)) != NULL) { \
value_ptr = value_ptr + strlen(key_ptr); \
tail_ptr = strchr(value_ptr, '\n'); \
tail_ptr[0] = '\0'; \
if(pkg_info->field == NULL) { pkg_info->field = vieter_package_dynarray_init(4); } \
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;
value_ptr = tail_ptr; \
} \
value_ptr = tail_ptr;
vieter_package_info *vieter_package_info_init() {
return calloc(1, sizeof(vieter_package_info));
@ -54,7 +61,8 @@ void vieter_package_info_free(vieter_package_info *pkg_info) {
free(pkg_info);
}
void vieter_package_info_parse(vieter_package_info *pkg_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);
@ -75,5 +83,4 @@ void vieter_package_info_parse(vieter_package_info *pkg_info, char *pkg_info_str
PKG_INFO_ARRAY("\noptdepend = ", optdepends);
PKG_INFO_ARRAY("\nmakedepend = ", makedepends);
PKG_INFO_ARRAY("\ncheckdepend = ", checkdepends);
}