From 437f4dd0c3548989562d28e5661dbb60d4408b9f Mon Sep 17 00:00:00 2001
From: picomeg <megordon5@gmail.com>
Date: Thu, 22 Oct 2020 03:28:32 +0100
Subject: [PATCH] cleaned up code with more error checking, more testing
---
src/hammer.c | 72 +++++++++++++++++++++------------------------------
src/t_names.c | 45 +++++++++++++++++++++++++++-----
2 files changed, 69 insertions(+), 48 deletions(-)
diff --git a/src/hammer.c b/src/hammer.c
index 0c6af368..64048197 100644
--- a/src/hammer.c
+++ b/src/hammer.c
@@ -329,62 +329,50 @@ char * h_get_short_name_with_no_params(HAllocator *mm__,
return h_get_backend_text_with_no_params(mm__, be, 0);
}
-/*TODO: refactor and clean this up more */
HParserBackendWithParams * h_get_backend_with_params_by_name(const char *name_with_params) {
-
- HParserBackendWithParams *result = NULL;
-
HAllocator *mm__ = &system_allocator;
-
+ HParserBackendWithParams *result = NULL;
+ HParserBackend be;
char *name_with_no_params = NULL;
char *params_as_string = NULL;
+ size_t name_len, len;
- size_t name_len;
- size_t len = strlen(name_with_params);
-
- result = h_new(HParserBackendWithParams, 1);
-
- if (result) {
+ if(name_with_params != NULL) {
+ result = h_new(HParserBackendWithParams, 1);
- params_as_string = strstr(name_with_params, "(");
+ if (result) {
+ len = strlen(name_with_params);
+ params_as_string = strstr(name_with_params, "(");
- if(params_as_string) {
- name_len = len - strlen(params_as_string);
- } else {
- name_len = len;
- }
+ if(params_as_string) {
+ name_len = len - strlen(params_as_string);
+ } else {
+ name_len = len;
+ }
- name_with_no_params = h_new(char, name_len+1);
- memset(name_with_no_params, '\0', name_len+1);
- strncpy(name_with_no_params, name_with_params, name_len);
+ name_with_no_params = h_new(char, name_len+1);
+ memset(name_with_no_params, '\0', name_len+1);
+ strncpy(name_with_no_params, name_with_params, name_len);
- HParserBackend backend = h_query_backend_by_name(name_with_no_params);
+ be = h_query_backend_by_name(name_with_no_params);
+ result->backend = be;
+ result->name = name_with_no_params;
- result->backend = backend;
-
- result->name = name_with_no_params;
-
- if(params_as_string) {
-
- //if the backend is one built as part of hammer, use it
- if(backend){
- int s = backends[backend]->extract_params(&(result->params), params_as_string);
- if (!s) {
- /* copy_params() failed */
- h_free(result);
- result = NULL;
- }
- }
-
- } else {
- /* else just ignore it and set it to NULL */
- result->params = NULL;
- }
+ /* use the backend supplied method to extract any params from the input */
+ result->params = NULL;
+ if(params_as_string) {
+ if (be >= PB_MIN && be <= PB_MAX && be != PB_INVALID &&
+ backends[be] != backends[PB_INVALID]) {
+ if (backends[be]->extract_params) {
+ backends[be]->extract_params(&(result->params), params_as_string);
+ }
+ }
+ }
+ }
}
-
return result;
}
diff --git a/src/t_names.c b/src/t_names.c
index 95cefdeb..b44ea70d 100644
--- a/src/t_names.c
+++ b/src/t_names.c
@@ -54,18 +54,54 @@ static void test_tt_query_backend_by_name(void) {
static void test_tt_get_backend_with_params_by_name(void) {
HParserBackendWithParams * be_w_p = NULL;
+ /*requests to use default params, or for backends without params*/
+ be_w_p = h_get_backend_with_params_by_name(packrat_name);
+ g_check_inttype("%d", HParserBackend, be_w_p->backend, ==, PB_PACKRAT);
+ g_check_maybe_string_eq(be_w_p->name, packrat_name);
+
be_w_p = h_get_backend_with_params_by_name(glr_name);
g_check_inttype("%d", HParserBackend, be_w_p->backend, ==, PB_GLR);
g_check_maybe_string_eq(be_w_p->name, glr_name);
+ /* request with params */
be_w_p = h_get_backend_with_params_by_name("glr(1)");
-
g_check_inttype("%d", HParserBackend, be_w_p->backend, ==, PB_GLR);
-
g_check_maybe_string_eq(be_w_p->name, glr_name);
-
g_check_cmp_size((uintptr_t)be_w_p->params, ==, 1);
+ /*request for default params - alternative possible style */
+ be_w_p = h_get_backend_with_params_by_name("glr()");
+ g_check_inttype("%d", HParserBackend, be_w_p->backend, ==, PB_GLR);
+ g_check_maybe_string_eq(be_w_p->name, glr_name);
+
+ /*request for a backend not in the enum of backends included in hammer */
+ be_w_p = h_get_backend_with_params_by_name("llvm");
+ g_check_inttype("%d", HParserBackend, be_w_p->backend, ==, PB_INVALID);
+ g_check_maybe_string_eq(be_w_p->name, "llvm");
+
+ /*tests ahowing that the parsing of names and params as currently written is
+ * fault tolerant to things like calling a backend which does not take params
+ * with params, sending extra params, or plain old garbage after the name or
+ * the valid params.
+ * If we choose to enforce sanity checks on input, these are the tests we should see
+ * break and then change to reflect those sanity checks*/
+ be_w_p = h_get_backend_with_params_by_name("packrat(0)");
+ g_check_inttype("%d", HParserBackend, be_w_p->backend, ==, PB_PACKRAT);
+ g_check_maybe_string_eq(be_w_p->name, packrat_name);
+
+ be_w_p = h_get_backend_with_params_by_name("glr(1,2)");
+ g_check_inttype("%d", HParserBackend, be_w_p->backend, ==, PB_GLR);
+ g_check_maybe_string_eq(be_w_p->name, glr_name);
+ g_check_cmp_size((uintptr_t)be_w_p->params, ==, 1);
+
+ be_w_p = h_get_backend_with_params_by_name("glr(1 vnvnvn");
+ g_check_inttype("%d", HParserBackend, be_w_p->backend, ==, PB_GLR);
+ g_check_maybe_string_eq(be_w_p->name, glr_name);
+ g_check_cmp_size((uintptr_t)be_w_p->params, ==, 1);
+
+ be_w_p = h_get_backend_with_params_by_name("glr(");
+ g_check_inttype("%d", HParserBackend, be_w_p->backend, ==, PB_GLR);
+ g_check_maybe_string_eq(be_w_p->name, glr_name);
}
@@ -86,9 +122,6 @@ static void test_tt_h_compile_for_named_backend(void) {
g_test_message("Compile failed");
g_test_fail();
}
-
- /* TODO further test for success? */
-
}
void register_names_tests(void) {
--
GitLab