From 8a5405629f7dcbc2504ac55f57775180a011b846 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yaroslav=20de=20la=20Pe=C3=B1a=20Smirnov?= Date: Sun, 7 Nov 2021 00:59:26 +0300 Subject: Fixes and improvements * Fix heap corruption on buffer growth. * Define as static functions that are not used outside a TU. * Other minor changes. --- README.md | 5 ++ src/hashmap.c | 15 +----- src/hashmap.h | 13 ++++- src/template.c | 158 +++++++++++++++++++++++++++++++++++++++++---------------- src/template.h | 4 +- src/vector.c | 45 +++++++++++----- src/vector.h | 14 +++-- 7 files changed, 177 insertions(+), 77 deletions(-) diff --git a/README.md b/README.md index a20da63..72cdadb 100644 --- a/README.md +++ b/README.md @@ -57,12 +57,17 @@ int main() { ### Done +* Use FNV1a as hashmap hashing algorithm. * Hashmap capacity can be set programmatically and it grows/shrinks in size when the load increases/decreases to avoid collisions/save memory. * Include guards. +* Fix heap corruption on buffer growth. +* Define as static functions that are not used outside a TU. ### TODO +* Handle empty/non-existent variables +* Handle non-existent templates * Let the library user decide how to handle errors, instead of just aborting the program. * Do not print anything to stdout/stderr. It should be up to the library user to diff --git a/src/hashmap.c b/src/hashmap.c index fa9a295..2522907 100644 --- a/src/hashmap.c +++ b/src/hashmap.c @@ -117,10 +117,6 @@ hashmap_new_with_cap(size_t cap) return hm; } -/* - * Inserts a key-value pair into the map. Returns NULL if map did not have key, - * old value if it did. - */ void * hashmap_insert(struct hashmap *hm, const char *key, void *value) { @@ -148,7 +144,6 @@ hashmap_insert(struct hashmap *hm, const char *key, void *value) return NULL; } -/* Returns a pointer to the value corresponding to the key. */ void * hashmap_get(struct hashmap *hm, const char *key) { @@ -165,7 +160,6 @@ hashmap_get(struct hashmap *hm, const char *key) return NULL; } -/* Retrieve pointer to value by key, handles dot notation for nested hashmaps */ void * hashmap_resolve(struct hashmap *hm, const char *key) { @@ -192,10 +186,6 @@ hashmap_resolve(struct hashmap *hm, const char *key) return hm; } -/* - * Removes a key from the map, returning the value at the key if the key was - * previously in the map. - */ void * hashmap_remove(struct hashmap *hm, const char *key) { @@ -226,7 +216,7 @@ hashmap_remove(struct hashmap *hm, const char *key) } void -hashmap_walk(struct hashmap *hm, void (*fn)(void *value)) +hashmap_walk(struct hashmap *hm, void (*fn)(const void *key, void *value)) { struct node *node; struct node *next; @@ -235,13 +225,12 @@ hashmap_walk(struct hashmap *hm, void (*fn)(void *value)) node = hm->buckets[i]; while (node) { next = node->next; - fn(node->value); + fn(node->key, node->value); node = next; } } } -/* free hashmap related memory */ void hashmap_free(struct hashmap *hm) { diff --git a/src/hashmap.h b/src/hashmap.h index 5a1e22d..981c21b 100644 --- a/src/hashmap.h +++ b/src/hashmap.h @@ -31,16 +31,27 @@ struct hashmap *hashmap_new_with_cap(size_t cap); #define hashmap_new() hashmap_new_with_cap(HASHMAP_CAP) +/* + * Inserts a key-value pair into the map. Returns NULL if map did not have key, + * old value if it did. + */ void *hashmap_insert(struct hashmap *hm, const char *key, void *value); +/* Returns a pointer to the value corresponding to the key. */ void *hashmap_get(struct hashmap *hm, const char *key); +/* Retrieve pointer to value by key, handles dot notation for nested hashmaps */ void *hashmap_resolve(struct hashmap *hm, const char *key); +/* + * Removes a key from the map, returning the value at the key if the key was + * previously in the map. + */ void *hashmap_remove(struct hashmap *hm, const char *key); +/* free hashmap related memory */ void hashmap_free(struct hashmap *hm); -void hashmap_walk(struct hashmap *hm, void (*fn)(void *value)); +void hashmap_walk(struct hashmap *hm, void (*fn)(const void *key, void *value)); #endif diff --git a/src/template.c b/src/template.c index a92edd6..f73ca1d 100644 --- a/src/template.c +++ b/src/template.c @@ -44,22 +44,34 @@ struct template { }; /* ensure buffer has room for a string sized l, grows buffer capacity if needed */ -void buffer_reserve(struct buffer *buf, int l) { +static void +buffer_reserve(struct buffer *buf, size_t l) +{ int req_size = buf->size + l; if (req_size >= buf->cap) { while (req_size >= buf->cap) { buf->cap *= 2; } - buf->string = realloc(buf->string, buf->cap * sizeof *buf->string); + buf->string = realloc(buf->string, buf->cap); if (!buf->string) { errx(EXIT_FAILURE, "out of memory"); } } } +static void +buffer_cat(struct buffer *buf, const char *str) +{ + size_t l = strlen(str); + buffer_reserve(buf, l); + strcat(buf->string, str); + buf->size += l; +} -mpc_parser_t *parser_init() { +static mpc_parser_t * +parser_init() +{ static mpc_parser_t *template; if (template != NULL) { return template; @@ -144,7 +156,9 @@ mpc_parser_t *parser_init() { return template; } -mpc_ast_t *parse(char *tmpl) { +static mpc_ast_t * +parse(char *tmpl) +{ mpc_parser_t *parser = parser_init(); mpc_result_t r; @@ -158,7 +172,9 @@ mpc_ast_t *parse(char *tmpl) { return r.output; } -struct hashmap *find_blocks_in_ast(mpc_ast_t *node, struct hashmap *map) { +static struct hashmap * +find_blocks_in_ast(mpc_ast_t *node, struct hashmap *map) +{ if (strstr(node->tag, "content|statement|block")) { char *name = node->children[2]->contents; hashmap_insert(map, name, node); @@ -171,17 +187,19 @@ struct hashmap *find_blocks_in_ast(mpc_ast_t *node, struct hashmap *map) { return map; } -struct env *env_new(char *dirname) { +struct env * +env_new(const char *dirname) +{ /* store current working dir so we can revert to it after reading templates */ char working_dir[256]; getcwd(working_dir, 255); DIR *dr = opendir(dirname); - if (dr == NULL) { - errx(EXIT_FAILURE, "could not open directory \"%s\"", dirname); - } + if (dr == NULL) return NULL; struct env *env = malloc(sizeof *env); + if (env == NULL) return NULL; + env->templates = hashmap_new(); chdir(dirname); @@ -221,21 +239,27 @@ struct env *env_new(char *dirname) { return env; } -void template_free(void *v) { - struct template *t = (struct template *)v; +static void +template_free(const void *k, void *v) +{ + struct template *t = v; hashmap_free(t->blocks); mpc_ast_delete(t->ast); free(t->name); free(t); } -void env_free(struct env *env) { +void +env_free(struct env *env) +{ hashmap_walk(env->templates, template_free); hashmap_free(env->templates); free(env); } -char *read_file(char *filename) { +char * +read_file(char *filename) +{ char *input = malloc(BUFSIZ); unsigned int size = 0; @@ -256,14 +280,18 @@ char *read_file(char *filename) { return input; } -char *trim_trailing_whitespace(char *str) { +static char * +trim_trailing_whitespace(char *str) +{ for (int i=strlen(str)-1; i >= 0 && isspace(str[i]); i--) { str[i] = '\0'; } return str; } -char *trim_leading_whitespace(char *str) { +static char * +trim_leading_whitespace(char *str) +{ while (isspace(*str)) { str++; } @@ -271,7 +299,9 @@ char *trim_leading_whitespace(char *str) { return str; } -struct unja_object *make_string_object(char *value, char *value2) { +static struct unja_object * +make_string_object(char *value, char *value2) +{ struct unja_object *obj = malloc(sizeof *obj); obj->type = OBJ_STRING; int l = strlen(value) + 1; @@ -290,7 +320,9 @@ struct unja_object *make_string_object(char *value, char *value2) { return obj; } -struct unja_object *make_int_object(int value) { +static struct unja_object * +make_int_object(int value) +{ struct unja_object *obj = malloc(sizeof *obj); obj->type = OBJ_INT; obj->integer = value; @@ -298,25 +330,27 @@ struct unja_object *make_int_object(int value) { } -void eval_object(struct buffer *buf, struct unja_object *obj) { +static void +eval_object(struct buffer *buf, struct unja_object *obj) +{ char tmp[64]; switch (obj->type) { case OBJ_NULL: break; case OBJ_STRING: - buffer_reserve(buf, strlen(obj->string)); - strcat(buf->string, obj->string); + buffer_cat(buf, obj->string); break; case OBJ_INT: sprintf(tmp, "%d", obj->integer); - buffer_reserve(buf, strlen(tmp)); - strcat(buf->string, tmp); + buffer_cat(buf, tmp); break; } } -int object_to_int(struct unja_object *obj) { +static int +object_to_int(struct unja_object *obj) +{ switch (obj->type) { case OBJ_NULL: return 0; case OBJ_STRING: return atoi(obj->string); @@ -326,7 +360,9 @@ int object_to_int(struct unja_object *obj) { return 0; } -void object_free(struct unja_object *obj) { +static void +object_free(struct unja_object *obj) +{ switch(obj->type) { case OBJ_NULL: return; case OBJ_STRING: @@ -338,7 +374,9 @@ void object_free(struct unja_object *obj) { free(obj); } -int object_is_truthy(struct unja_object *obj) { +static int +object_is_truthy(struct unja_object *obj) +{ switch (obj->type) { case OBJ_NULL: return 0; case OBJ_STRING: return strlen(obj->string) > 0 && strcmp(obj->string, "0") != 0; @@ -355,7 +393,9 @@ struct context { struct template *current_template; }; -struct unja_object *eval_expression_value(mpc_ast_t* node, struct context *ctx) { +static struct unja_object * +eval_expression_value(mpc_ast_t* node, struct context *ctx) +{ if (strstr(node->tag, "symbol|")) { /* Return empty string if no vars were passed. Should probably signal error here. */ if (ctx->vars == NULL) { @@ -380,7 +420,9 @@ struct unja_object *eval_expression_value(mpc_ast_t* node, struct context *ctx) return &null_object; } -struct unja_object *eval_string_infix_expression(struct unja_object *left, char *op, struct unja_object *right) { +static struct unja_object * +eval_string_infix_expression(struct unja_object *left, char *op, struct unja_object *right) +{ struct unja_object *result; if (strcmp(op, "+") == 0) { @@ -398,7 +440,9 @@ struct unja_object *eval_string_infix_expression(struct unja_object *left, char return result; } -struct unja_object *eval_infix_expression(struct unja_object *left, char *op, struct unja_object *right) { +static struct unja_object * +eval_infix_expression(struct unja_object *left, char *op, struct unja_object *right) +{ /* if operator is + and either left or right node is of type string: concat */ if (left->type == OBJ_STRING && right->type == OBJ_STRING) { return eval_string_infix_expression(left, op, right); @@ -446,7 +490,9 @@ struct unja_object *eval_infix_expression(struct unja_object *left, char *op, st return make_int_object(result); } -struct unja_object *eval_expression(mpc_ast_t* expr, struct context *ctx) { +static struct unja_object * +eval_expression(mpc_ast_t* expr, struct context *ctx) +{ /* singular term */ if (expr->children_num == 0 || strstr(expr->tag, "string|")) { @@ -495,7 +541,9 @@ struct unja_object *eval_expression(mpc_ast_t* expr, struct context *ctx) { } -int eval(struct buffer *buf, mpc_ast_t* t, struct context *ctx) { +static int +eval(struct buffer *buf, mpc_ast_t* t, struct context *ctx) +{ static int trim_whitespace = 0; // maybe eat whitespace going backward @@ -510,8 +558,7 @@ int eval(struct buffer *buf, mpc_ast_t* t, struct context *ctx) { trim_whitespace = 0; } - buffer_reserve(buf, strlen(str)); - strcat(buf->string, str); + buffer_cat(buf, str); return 0; } @@ -534,7 +581,9 @@ int eval(struct buffer *buf, mpc_ast_t* t, struct context *ctx) { eval(buf, t->children[4], ctx); } - trim_whitespace = strstr(t->children[7]->contents, "-") ? 1 : 0; + if (t->children_num == 8) { + trim_whitespace = strstr(t->children[7]->contents, "-") ? 1 : 0; + } return 0; } @@ -627,7 +676,9 @@ int eval(struct buffer *buf, mpc_ast_t* t, struct context *ctx) { return 0; } -char *render_ast(mpc_ast_t *ast, struct context *ctx) { +static char * +render_ast(mpc_ast_t *ast, struct context *ctx) +{ #if DEBUG printf("AST: \n"); mpc_ast_print(ast); @@ -643,14 +694,18 @@ char *render_ast(mpc_ast_t *ast, struct context *ctx) { return buf.string; } -struct unja_object *filter_trim(struct unja_object *obj) { +static struct unja_object * +filter_trim(struct unja_object *obj) +{ assert(obj->type == OBJ_STRING); obj->string = trim_leading_whitespace(obj->string); trim_trailing_whitespace(obj->string); return obj; } -struct unja_object *filter_lower(struct unja_object *obj) { +static struct unja_object * +filter_lower(struct unja_object *obj) +{ assert(obj->type == OBJ_STRING); int len = strlen(obj->string); for (int i=0; i < len; i++) { @@ -659,7 +714,9 @@ struct unja_object *filter_lower(struct unja_object *obj) { return obj; } -struct unja_object *filter_wordcount(struct unja_object *obj) { +static struct unja_object * +filter_wordcount(struct unja_object *obj) +{ assert(obj->type == OBJ_STRING); int len = strlen(obj->string); int word_count = 1; @@ -674,14 +731,18 @@ struct unja_object *filter_wordcount(struct unja_object *obj) { } -struct unja_object *filter_length(struct unja_object *obj) { +static struct unja_object * +filter_length(struct unja_object *obj) +{ assert(obj->type == OBJ_STRING); int len = strlen(obj->string); object_free(obj); return make_int_object(len); } -struct hashmap *default_filters() { +static struct hashmap * +default_filters() +{ struct hashmap *filters = hashmap_new(); hashmap_insert(filters, "trim", filter_trim); hashmap_insert(filters, "lower", filter_lower); @@ -690,7 +751,9 @@ struct hashmap *default_filters() { return filters; } -struct context context_new(struct hashmap *vars, struct env *env, struct template *current_tmpl) { +static struct context +context_new(struct hashmap *vars, struct env *env, struct template *current_tmpl) +{ struct context ctx; ctx.filters = default_filters(); ctx.vars = vars; @@ -699,11 +762,15 @@ struct context context_new(struct hashmap *vars, struct env *env, struct templat return ctx; } -void context_free(struct context ctx) { +static void +context_free(struct context ctx) +{ hashmap_free(ctx.filters); } -char *template_string(char *tmpl, struct hashmap *vars) { +char * +template_string(char *tmpl, struct hashmap *vars) +{ #if DEBUG printf("Template: %s\n", tmpl); #endif @@ -715,8 +782,11 @@ char *template_string(char *tmpl, struct hashmap *vars) { return output; } -char *template(struct env *env, char *template_name, struct hashmap *vars) { +char * +template(struct env *env, const char *template_name, struct hashmap *vars) +{ struct template *t = hashmap_get(env->templates, template_name); + /* TODO: handle template not found */ #if DEBUG printf("Template name: %s\n", t->name); printf("Parent: %s\n", t->parent ? t->parent : "None"); @@ -739,4 +809,4 @@ char *template(struct env *env, char *template_name, struct hashmap *vars) { char *output = render_ast(t->ast, &ctx); context_free(ctx); return output; -} \ No newline at end of file +} diff --git a/src/template.h b/src/template.h index 6defb02..cf21726 100644 --- a/src/template.h +++ b/src/template.h @@ -6,11 +6,11 @@ struct env; -struct env *env_new(); +struct env *env_new(const char *dirname); void env_free(struct env *env); -char *template(struct env *env, char *template_name, struct hashmap *ctx); +char *template(struct env *env, const char *template_name, struct hashmap *ctx); char *template_string(char *tmpl, struct hashmap *ctx); diff --git a/src/vector.c b/src/vector.c index 46a65e5..536fd33 100644 --- a/src/vector.c +++ b/src/vector.c @@ -1,23 +1,44 @@ -#include #include "vector.h" +#include +#include + +static bool +vector_grow(struct vector *vec) +{ + vec->cap *= VECTOR_GROW_RATE; + vec->values = realloc(vec->values, sizeof(*vec->values) * vec->cap); + if (vec->values == NULL) { + return false; + } + + return true; +} + /* create a new vector of the given capacity */ -struct vector* vector_new(int cap) { - struct vector *l = malloc(sizeof *l); - l->size = 0; - l->cap = cap; - l->values = malloc(l->cap * sizeof *l->values); - return l; +struct vector * +vector_new(size_t cap) +{ + struct vector *vec = malloc(sizeof *vec); + vec->size = 0; + vec->cap = cap; + vec->values = malloc(vec->cap * sizeof *vec->values); + return vec; } /* push a new value to the end of the vector's memory */ -int vector_push(struct vector *vec, void *value) { +size_t +vector_push(struct vector *vec, void *value) +{ + if (vec->size == vec->cap) vector_grow(vec); vec->values[vec->size++] = value; return vec->size - 1; } /* free vector related memory */ -void vector_free(struct vector *l) { - free(l->values); - free(l); -} \ No newline at end of file +void +vector_free(struct vector *vec) +{ + free(vec->values); + free(vec); +} diff --git a/src/vector.h b/src/vector.h index 0a7c297..8df3af1 100644 --- a/src/vector.h +++ b/src/vector.h @@ -1,17 +1,21 @@ #ifndef UNJA_VECTOR_H #define UNJA_VECTOR_H -#include +#include + +#ifndef VECTOR_GROW_RATE +#define VECTOR_GROW_RATE 2 +#endif struct vector { void **values; - int size; - int cap; + size_t size; + size_t cap; }; -struct vector* vector_new(int cap); +struct vector *vector_new(size_t cap); -int vector_push(struct vector *vec, void *value); +size_t vector_push(struct vector *vec, void *value); void vector_free(struct vector *vec); -- cgit v1.2.3