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 +33 -28
2 changed files with 33 additions and 28 deletions
Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit c7c4043108 - Show all commits

10
include/vieter_package.h
View file

@ -1,16 +1,10 @@
#ifndef VIETER_PACKAGE #ifndef VIETER_PACKAGE
#define VIETER_PACKAGE #define VIETER_PACKAGE
#include <stdint.h>
#include <stdbool.h>
#include <string.h>
#include <stdlib.h>
#include <archive.h>
#include <archive_entry.h>
typedef struct vieter_package vieter_package; typedef struct vieter_package vieter_package;
/* /*
* Parse package file into something usable by libvieter. * Parse package file into something usable by libvieter.
* The pointer returned by this function will need to freed at a later point. * The pointer returned by this function will need to freed at a later point.

51
src/package/vieter_package.c
View file

@ -1,30 +1,29 @@
#include <stdlib.h>
#include <stdbool.h>
#include <archive.h>
#include <archive_entry.h>
#include "vieter_package_internal.h" #include "vieter_package_internal.h"
#include "sha256.h" #include "sha256.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 SMALL_BUFF_SIZE 128
#define ADD_STRING(section, field) if (pkg_info->field != 0) { \ #define ADD_STRING(section, field) if (pkg_info->field != 0) { \
snprintf(aux, SMALL_BUFF_SIZE, section, pkg_info->field); \ size_to_be_written = snprintf(aux, small_buff_size, section, pkg_info->field); \
if (buff_size < strlen(description) + SMALL_BUFF_SIZE + 1) { \ 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, section, pkg_info->field); \
} \
if (buff_size < strlen(description) + small_buff_size + 1) { \
description = realloc(description, buff_size * 2); \ description = realloc(description, buff_size * 2); \
buff_size *= 2; \ buff_size *= 2; \
} \ } \
strcat(description, aux); \ strcat(description, aux); \
} }
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.
#define ADD_ARRAY(section, field) i = 0; if (pkg_info->field != NULL) { \ #define ADD_ARRAY(section, field) i = 0; if (pkg_info->field != NULL) { \
snprintf(aux, SMALL_BUFF_SIZE, section, pkg_info->field->array[i]); i++; \ ADD_STRING(section, 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 (pkg_info->field->array[i] != NULL) { \ while (pkg_info->field->array[i] != NULL) { \
snprintf(aux, SMALL_BUFF_SIZE, "\n%s", pkg_info->field->array[i]); i++; \ ADD_STRING("\n%s", 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); \
} \ } \
} }
@ -170,19 +169,32 @@ void vieter_package_sha256sum(vieter_package *pkg, char *res) {
res[j] = '\0'; res[j] = '\0';
} }
char *vieter_package_to_description(vieter_package *pkg) { char *vieter_package_to_description(vieter_package *pkg) {
vieter_package_info *pkg_info = pkg->info; vieter_package_info *pkg_info = pkg->info;
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.
size_t buff_size = 1024; size_t buff_size = 1024;
char aux[SMALL_BUFF_SIZE]; int small_buff_size = 128;
int size_to_be_written;
char *aux = malloc(sizeof(char) * small_buff_size);
char *description = malloc(sizeof(char) * buff_size); char *description = malloc(sizeof(char) * buff_size);
// Helper variable for ADD_ARRAY macro // Helper variable for ADD_ARRAY macro
int i; int i;
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.
// special case for FILENAME // special case for FILENAME
// assuming .pkg.tar.zst; other formats are valid, this should account for that // 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", pkg_info->name, 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); 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);
}
strcpy(description, aux); strcpy(description, aux);
ADD_STRING("\n\n%%NAME%%\n%s", name); ADD_STRING("\n\n%%NAME%%\n%s", name);
@ -216,8 +228,7 @@ char *vieter_package_to_description(vieter_package *pkg) {
ADD_ARRAY("\n\n%%MAKEDEPENDS%%\n%s", makedepends); ADD_ARRAY("\n\n%%MAKEDEPENDS%%\n%s", makedepends);
ADD_ARRAY("\n\n%%CHECKDEPENDS%%\n%s", checkdepends); ADD_ARRAY("\n\n%%CHECKDEPENDS%%\n%s", checkdepends);
snprintf(aux, SMALL_BUFF_SIZE, "\n\n"); strcat(description, "\n\n");
strcat(description, aux);
return description; return description;
} }