refactor: centralise matrix ownership and request cleanup

src/Routes/Images.c

@@ -41,6 +41,10 @@ static char *images_href_builder(int page, void *data) {
   return build_images_href((const char *)data, page);
 }
 
+static void free_pagination(char ***matrix, int *inner_counts, int count) {
+  free_string_matrix(matrix, inner_counts, count);
+}
+
 int images_handler(UrlParams *params) {
   extern Config global_config;
   TemplateContext ctx = new_context();
@@ -91,8 +95,9 @@ int images_handler(UrlParams *params) {
 
   if (!raw_query || strlen(raw_query) == 0) {
     send_redirect("/");
-    if (display_query)
+    free_pagination(pager_matrix, pager_inner_counts, pager_count);
     free(display_query);
+    free(locale);
     free_context(&ctx);
     return -1;
   }
@@ -128,7 +133,9 @@ int images_handler(UrlParams *params) {
       snprintf(response, sizeof(response), "<h1>%s</h1>", rate_limit_msg);
       send_response(response);
       free(request_cache_key);
+      free_pagination(pager_matrix, pager_inner_counts, pager_count);
       free(display_query);
+      free(locale);
       free_context(&ctx);
       return -1;
     }
@@ -147,36 +154,41 @@ int images_handler(UrlParams *params) {
     snprintf(error_html, sizeof(error_html), "<h1>%s</h1>", error_images_msg);
     send_response(error_html);
     free(request_cache_key);
+    free_pagination(pager_matrix, pager_inner_counts, pager_count);
     free(display_query);
+    free(locale);
     free_context(&ctx);
     return -1;
   }
 
-  char ***image_matrix = malloc(sizeof(char **) * result_count);
+  char ***image_matrix = NULL;
-  int *inner_counts = malloc(sizeof(int) * result_count);
+  int *inner_counts = NULL;
+  int image_count = 0;
 
-  if (!image_matrix || !inner_counts) {
+  for (int i = 0; i < result_count; i++) {
-    if (image_matrix)
+    const char *values[IMAGE_RESULT_FIELDS] = {
-      free(image_matrix);
+        results[i].thumbnail_url,
-    if (inner_counts)
+        results[i].title,
-      free(inner_counts);
+        results[i].page_url,
+        results[i].full_url,
+    };
+    int new_count = append_string_row(&image_matrix, &inner_counts,
+                                      image_count, values,
+                                      IMAGE_RESULT_FIELDS);
+    if (new_count == image_count) {
+      free_string_matrix(image_matrix, inner_counts, image_count);
+      free_pagination(pager_matrix, pager_inner_counts, pager_count);
       free_image_results(results, result_count);
       free(request_cache_key);
       free(display_query);
+      free(locale);
       free_context(&ctx);
       return -1;
     }
-
+    image_count = new_count;
-  for (int i = 0; i < result_count; i++) {
-    image_matrix[i] = malloc(sizeof(char *) * IMAGE_RESULT_FIELDS);
-    image_matrix[i][0] = strdup(results[i].thumbnail_url);
-    image_matrix[i][1] = strdup(results[i].title);
-    image_matrix[i][2] = strdup(results[i].page_url);
-    image_matrix[i][3] = strdup(results[i].full_url);
-    inner_counts[i] = IMAGE_RESULT_FIELDS;
   }
 
-  context_set_array_of_arrays(&ctx, "images", image_matrix, result_count,
+  context_set_array_of_arrays(&ctx, "images", image_matrix, image_count,
                               inner_counts);
 
   char *rendered = render_template("images.html", &ctx);
@@ -187,27 +199,14 @@ int images_handler(UrlParams *params) {
     send_response("<h1>Error rendering image results</h1>");
   }
 
-  for (int i = 0; i < result_count; i++) {
+  free_string_matrix(image_matrix, inner_counts, image_count);
-    for (int j = 0; j < IMAGE_RESULT_FIELDS; j++)
-      free(image_matrix[i][j]);
-    free(image_matrix[i]);
-  }
-  free(image_matrix);
-  free(inner_counts);
 
-  if (pager_count > 0) {
+  free_pagination(pager_matrix, pager_inner_counts, pager_count);
-    for (int i = 0; i < pager_count; i++) {
-      for (int j = 0; j < LINK_FIELD_COUNT; j++)
-        free(pager_matrix[i][j]);
-      free(pager_matrix[i]);
-    }
-    free(pager_matrix);
-    free(pager_inner_counts);
-  }
 
   free_image_results(results, result_count);
   free(request_cache_key);
   free(display_query);
+  free(locale);
   free_context(&ctx);
 
   return 0;

src/Routes/Search.c

@@ -251,71 +251,49 @@ static void *infobox_thread_func(void *arg) {
 
 static int add_infobox_to_collection(InfoBox *infobox, char ****collection,
                                      int **inner_counts, int current_count) {
-  *collection =
+  const char *values[INFOBOX_FIELD_COUNT] = {
-      (char ***)realloc(*collection, sizeof(char **) * (current_count + 1));
+      infobox->title,
-  *inner_counts =
+      infobox->thumbnail_url,
-      (int *)realloc(*inner_counts, sizeof(int) * (current_count + 1));
+      infobox->extract,
-
+      infobox->url,
-  (*collection)[current_count] =
+      infobox->url,
-      (char **)malloc(sizeof(char *) * INFOBOX_FIELD_COUNT);
+  };
-  (*collection)[current_count][0] =
+  return append_string_row(collection, inner_counts, current_count, values,
-      infobox->title ? strdup(infobox->title) : NULL;
+                           INFOBOX_FIELD_COUNT);
-  (*collection)[current_count][1] =
-      infobox->thumbnail_url ? strdup(infobox->thumbnail_url) : NULL;
-  (*collection)[current_count][2] =
-      infobox->extract ? strdup(infobox->extract) : NULL;
-  (*collection)[current_count][3] = infobox->url ? strdup(infobox->url) : NULL;
-  (*collection)[current_count][4] = infobox->url ? strdup(infobox->url) : NULL;
-  (*inner_counts)[current_count] = INFOBOX_FIELD_COUNT;
-
-  return current_count + 1;
 }
 
 static int add_warning_to_collection(const char *engine_name,
                                      const char *warning_message,
                                      char ****collection, int **inner_counts,
                                      int current_count) {
-  char ***new_collection =
+  const char *values[] = {engine_name, warning_message};
-      (char ***)malloc(sizeof(char **) * (current_count + 1));
+  return append_string_row(collection, inner_counts, current_count, values,
-  int *new_inner_counts =
+                           2);
-      (int *)malloc(sizeof(int) * (current_count + 1));
+}
 
-  if (!new_collection || !new_inner_counts) {
+static void free_user_engine_list(char **user_engines, int user_engine_count) {
-    free(new_collection);
+  for (int i = 0; i < user_engine_count; i++)
-    free(new_inner_counts);
+    free(user_engines[i]);
-    return current_count;
+  free(user_engines);
+}
+
+static void free_search_results(SearchResult **all_results, ScrapeJob *jobs,
+                                int job_count) {
+  for (int i = 0; i < job_count; i++) {
+    for (int j = 0; all_results[i] && j < jobs[i].results_count; j++) {
+      free(all_results[i][j].url);
+      free(all_results[i][j].title);
+      free(all_results[i][j].snippet);
     }
-
+    free(all_results[i]);
-  if (*collection && current_count > 0) {
-    memcpy(new_collection, *collection, sizeof(char **) * current_count);
   }
-  if (*inner_counts && current_count > 0) {
+}
-    memcpy(new_inner_counts, *inner_counts, sizeof(int) * current_count);
+
+static void free_infobox_results(InfoBoxThreadData *data) {
+  for (int i = 0; i < HANDLER_COUNT; i++) {
+    if (data[i].success)
+      free_infobox(&data[i].result);
   }
-
-  free(*collection);
-  free(*inner_counts);
-
-  *collection = new_collection;
-  *inner_counts = new_inner_counts;
-
-  (*collection)[current_count] = (char **)malloc(sizeof(char *) * 2);
-  if (!(*collection)[current_count])
-    return current_count;
-
-  (*collection)[current_count][0] = strdup(engine_name ? engine_name : "");
-  (*collection)[current_count][1] =
-      strdup(warning_message ? warning_message : "");
-
-  if (!(*collection)[current_count][0] || !(*collection)[current_count][1]) {
-    free((*collection)[current_count][0]);
-    free((*collection)[current_count][1]);
-    free((*collection)[current_count]);
-    return current_count;
-  }
-
-  (*inner_counts)[current_count] = 2;
-  return current_count + 1;
 }
 
 static const char *warning_message_for_job(const ScrapeJob *job, const char *locale) {
@@ -491,11 +469,8 @@ int results_handler(UrlParams *params) {
 
   if (!raw_query || strlen(raw_query) == 0) {
     send_redirect("/");
-    if (has_user_pref) {
+    free(locale);
-      for (int i = 0; i < user_engine_count; i++)
+    free_user_engine_list(user_engines, user_engine_count);
-        free(user_engines[i]);
-      free(user_engines);
-    }
     free_context(&ctx);
     return -1;
   }
@@ -528,13 +503,6 @@ int results_handler(UrlParams *params) {
     infobox_data[i].result = (InfoBox){NULL};
   }
 
-  if (page == 1) {
-    for (int i = 0; i < HANDLER_COUNT; i++) {
-      pthread_create(&infobox_threads[i], NULL, infobox_thread_func,
-                     &infobox_data[i]);
-    }
-  }
-
   ScrapeJob jobs[ENGINE_COUNT];
   SearchResult *all_results[ENGINE_COUNT];
 
@@ -592,11 +560,8 @@ int results_handler(UrlParams *params) {
       snprintf(response, sizeof(response), "<h1>%s</h1>", rate_limit_msg);
       send_response(response);
       free(request_cache_key);
-      if (has_user_pref) {
+      free(locale);
-        for (int i = 0; i < user_engine_count; i++)
+      free_user_engine_list(user_engines, user_engine_count);
-          free(user_engines[i]);
-        free(user_engines);
-      }
       free_context(&ctx);
       return -1;
     }
@@ -648,13 +613,14 @@ int results_handler(UrlParams *params) {
     if (filter_count > 0) {
       context_set_array_of_arrays(&ctx, "engine_filters", filter_matrix,
                                   filter_count, filter_inner_counts);
-      for (int i = 0; i < filter_count; i++) {
+      free_string_matrix(filter_matrix, filter_inner_counts, filter_count);
-        for (int j = 0; j < LINK_FIELD_COUNT; j++)
-          free(filter_matrix[i][j]);
-        free(filter_matrix[i]);
     }
-      free(filter_matrix);
+  }
-      free(filter_inner_counts);
+
+  if (page == 1) {
+    for (int i = 0; i < HANDLER_COUNT; i++) {
+      pthread_create(&infobox_threads[i], NULL, infobox_thread_func,
+                     &infobox_data[i]);
     }
   }
 
@@ -672,27 +638,12 @@ int results_handler(UrlParams *params) {
     for (int i = 0; i < engine_idx; i++) {
       if (jobs[i].results_count > 0 && all_results[i][0].url) {
         char *redirect_url = strdup(all_results[i][0].url);
-        for (int j = 0; j < enabled_engine_count; j++) {
+        free_search_results(all_results, jobs, engine_idx);
-          for (int k = 0; k < jobs[j].results_count; k++) {
+        if (page == 1)
-            free(all_results[j][k].url);
+          free_infobox_results(infobox_data);
-            free(all_results[j][k].title);
-            free(all_results[j][k].snippet);
-          }
-          free(all_results[j]);
-        }
-        if (page == 1) {
-          for (int j = 0; j < HANDLER_COUNT; j++) {
-            if (infobox_data[j].success) {
-              free_infobox(&infobox_data[j].result);
-            }
-          }
-        }
         free(request_cache_key);
-        if (has_user_pref) {
+        free(locale);
-          for (int i = 0; i < user_engine_count; i++)
+        free_user_engine_list(user_engines, user_engine_count);
-            free(user_engines[i]);
-          free(user_engines);
-        }
         free_context(&ctx);
         if (redirect_url) {
           send_redirect(redirect_url);
@@ -701,22 +652,12 @@ int results_handler(UrlParams *params) {
         return 0;
       }
     }
-    for (int i = 0; i < enabled_engine_count; i++) {
+    free_search_results(all_results, jobs, engine_idx);
-      free(all_results[i]);
+    if (page == 1)
-    }
+      free_infobox_results(infobox_data);
-    if (page == 1) {
-      for (int i = 0; i < HANDLER_COUNT; i++) {
-        if (infobox_data[i].success) {
-          free_infobox(&infobox_data[i].result);
-        }
-      }
-    }
     free(request_cache_key);
-    if (has_user_pref) {
+    free(locale);
-      for (int i = 0; i < user_engine_count; i++)
+    free_user_engine_list(user_engines, user_engine_count);
-        free(user_engines[i]);
-      free(user_engines);
-    }
     free_context(&ctx);
     char no_results_html[128];
     snprintf(no_results_html, sizeof(no_results_html), "<h1>%s</h1>", no_results_msg);
@@ -741,13 +682,7 @@ int results_handler(UrlParams *params) {
   if (infobox_count > 0) {
     context_set_array_of_arrays(&ctx, "infoboxes", infobox_matrix,
                                 infobox_count, infobox_inner_counts);
-    for (int i = 0; i < infobox_count; i++) {
+    free_string_matrix(infobox_matrix, infobox_inner_counts, infobox_count);
-      for (int j = 0; j < INFOBOX_FIELD_COUNT; j++)
-        free(infobox_matrix[i][j]);
-      free(infobox_matrix[i]);
-    }
-    free(infobox_matrix);
-    free(infobox_inner_counts);
   }
 
   int warning_count = 0;
@@ -776,16 +711,7 @@ int results_handler(UrlParams *params) {
                                   warning_index, warning_inner_counts);
     }
 
-    if (warning_matrix) {
+    free_string_matrix(warning_matrix, warning_inner_counts, warning_index);
-      for (int i = 0; i < warning_index; i++) {
-        free(warning_matrix[i][0]);
-        free(warning_matrix[i][1]);
-        free(warning_matrix[i]);
-      }
-      free(warning_matrix);
-    }
-    if (warning_inner_counts)
-      free(warning_inner_counts);
   }
 
   int total_results = 0;
@@ -794,32 +720,8 @@ int results_handler(UrlParams *params) {
   }
 
   if (total_results > 0) {
-    char ***results_matrix = (char ***)malloc(sizeof(char **) * total_results);
+    char ***results_matrix = NULL;
-    int *results_inner_counts = (int *)malloc(sizeof(int) * total_results);
+    int *results_inner_counts = NULL;
-    if (!results_matrix || !results_inner_counts) {
-      char *html = render_template("results.html", &ctx);
-      if (html) {
-        send_response(html);
-        free(html);
-      }
-      for (int i = 0; i < enabled_engine_count; i++)
-        free(all_results[i]);
-      if (page == 1) {
-        for (int i = 0; i < HANDLER_COUNT; i++) {
-          if (infobox_data[i].success) {
-            free_infobox(&infobox_data[i].result);
-          }
-        }
-      }
-      free(request_cache_key);
-      if (has_user_pref) {
-        for (int i = 0; i < user_engine_count; i++)
-          free(user_engines[i]);
-        free(user_engines);
-      }
-      free_context(&ctx);
-      return 0;
-    }
     int unique_count = 0;
     UrlHashTable url_table;
     url_hash_init(&url_table);
@@ -837,37 +739,26 @@ int results_handler(UrlParams *params) {
 
         url_hash_insert(&url_table, display_url);
 
-        results_matrix[unique_count] =
-            (char **)malloc(sizeof(char *) * RESULT_FIELD_COUNT);
-        if (!results_matrix[unique_count]) {
-          free(all_results[i][j].url);
-          free(all_results[i][j].title);
-          free(all_results[i][j].snippet);
-          continue;
-        }
         char *pretty_url = pretty_display_url(display_url);
         char *base_url = get_base_url(display_url);
-
+        const char *values[RESULT_FIELD_COUNT] = {
-        results_matrix[unique_count][0] = strdup(display_url);
+            display_url,
-        results_matrix[unique_count][1] = strdup(pretty_url);
+            pretty_url,
-        results_matrix[unique_count][2] = all_results[i][j].title
+            all_results[i][j].title ? all_results[i][j].title : "Untitled",
-                                              ? strdup(all_results[i][j].title)
+            all_results[i][j].snippet ? all_results[i][j].snippet : "",
-                                              : strdup("Untitled");
+            base_url,
-        results_matrix[unique_count][3] =
+            "",
-            all_results[i][j].snippet ? strdup(all_results[i][j].snippet)
+        };
-                                      : strdup("");
+        int new_count = append_string_row(&results_matrix, &results_inner_counts,
-        results_matrix[unique_count][4] = strdup(base_url ? base_url : "");
+                                          unique_count, values,
-        results_matrix[unique_count][5] = strdup("");
+                                          RESULT_FIELD_COUNT);
-
-        results_inner_counts[unique_count] = RESULT_FIELD_COUNT;
 
         free(pretty_url);
         free(base_url);
         free(all_results[i][j].url);
         free(all_results[i][j].title);
         free(all_results[i][j].snippet);
-
+        unique_count = new_count;
-        unique_count++;
       }
       free(all_results[i]);
     }
@@ -885,13 +776,7 @@ int results_handler(UrlParams *params) {
     if (pager_count > 0) {
       context_set_array_of_arrays(&ctx, "pagination_links", pager_matrix,
                                   pager_count, pager_inner_counts);
-      for (int i = 0; i < pager_count; i++) {
+      free_string_matrix(pager_matrix, pager_inner_counts, pager_count);
-        for (int j = 0; j < LINK_FIELD_COUNT; j++)
-          free(pager_matrix[i][j]);
-        free(pager_matrix[i]);
-      }
-      free(pager_matrix);
-      free(pager_inner_counts);
     }
 
     char *html = render_template("results.html", &ctx);
@@ -900,13 +785,7 @@ int results_handler(UrlParams *params) {
       free(html);
     }
 
-    for (int i = 0; i < unique_count; i++) {
+    free_string_matrix(results_matrix, results_inner_counts, unique_count);
-      for (int j = 0; j < RESULT_FIELD_COUNT; j++)
-        free(results_matrix[i][j]);
-      free(results_matrix[i]);
-    }
-    free(results_matrix);
-    free(results_inner_counts);
     url_hash_free(&url_table);
   } else {
     char *html = render_template("results.html", &ctx);
@@ -915,26 +794,15 @@ int results_handler(UrlParams *params) {
       free(html);
     }
 
-    for (int i = 0; i < enabled_engine_count; i++) {
+    free_search_results(all_results, jobs, engine_idx);
-      free(all_results[i]);
-    }
   }
 
   free(request_cache_key);
 
-  if (page == 1) {
+  if (page == 1)
-    for (int i = 0; i < HANDLER_COUNT; i++) {
+    free_infobox_results(infobox_data);
-      if (infobox_data[i].success) {
-        free_infobox(&infobox_data[i].result);
-      }
-    }
-  }
   free(locale);
-  if (has_user_pref) {
+  free_user_engine_list(user_engines, user_engine_count);
-    for (int i = 0; i < user_engine_count; i++)
-      free(user_engines[i]);
-    free(user_engines);
-  }
   free_context(&ctx);
 
   return 0;

src/Utility/Utility.c

@@ -206,63 +206,69 @@ int user_engines_contains(const char *engine_id, char **ids, int count) {
   return 0;
 }
 
+int append_string_row(char ****matrix, int **inner_counts, int row_count,
+                      const char *const *values, int field_count) {
+  if (!matrix || !inner_counts || !values || field_count <= 0)
+    return row_count;
+
+  char **row = calloc((size_t)field_count, sizeof(*row));
+  if (!row)
+    return row_count;
+
+  for (int i = 0; i < field_count; i++) {
+    row[i] = strdup(values[i] ? values[i] : "");
+    if (!row[i]) {
+      for (int j = 0; j < i; j++)
+        free(row[j]);
+      free(row);
+      return row_count;
+    }
+  }
+
+  char ***new_matrix = malloc(sizeof(*new_matrix) * (row_count + 1));
+  int *new_counts = malloc(sizeof(*new_counts) * (row_count + 1));
+  if (!new_matrix || !new_counts) {
+    free(new_matrix);
+    free(new_counts);
+    for (int i = 0; i < field_count; i++)
+      free(row[i]);
+    free(row);
+    return row_count;
+  }
+
+  if (row_count > 0) {
+    memcpy(new_matrix, *matrix, sizeof(*new_matrix) * row_count);
+    memcpy(new_counts, *inner_counts, sizeof(*new_counts) * row_count);
+  }
+
+  new_matrix[row_count] = row;
+  new_counts[row_count] = field_count;
+  free(*matrix);
+  free(*inner_counts);
+  *matrix = new_matrix;
+  *inner_counts = new_counts;
+  return row_count + 1;
+}
+
+void free_string_matrix(char ***matrix, int *inner_counts, int row_count) {
+  if (matrix) {
+    for (int i = 0; i < row_count; i++) {
+      int field_count = inner_counts ? inner_counts[i] : 0;
+      for (int j = 0; j < field_count; j++)
+        free(matrix[i][j]);
+      free(matrix[i]);
+    }
+  }
+  free(matrix);
+  free(inner_counts);
+}
+
 int add_link_to_collection(const char *href, const char *label,
                            const char *class_name, char ****collection,
                            int **inner_counts, int current_count) {
-  char ***old_collection = *collection;
+  const char *values[LINK_FIELD_COUNT] = {href, label, class_name};
-  int *old_inner_counts = *inner_counts;
+  return append_string_row(collection, inner_counts, current_count, values,
-  char ***new_collection =
+                           LINK_FIELD_COUNT);
-      (char ***)malloc(sizeof(char **) * (current_count + 1));
-  int *new_inner_counts = (int *)malloc(sizeof(int) * (current_count + 1));
-
-  if (!new_collection || !new_inner_counts) {
-    free(new_collection);
-    free(new_inner_counts);
-    return current_count;
-  }
-
-  if (*collection && current_count > 0) {
-    memcpy(new_collection, *collection, sizeof(char **) * current_count);
-  }
-  if (*inner_counts && current_count > 0) {
-    memcpy(new_inner_counts, *inner_counts, sizeof(int) * current_count);
-  }
-
-  *collection = new_collection;
-  *inner_counts = new_inner_counts;
-
-  (*collection)[current_count] =
-      (char **)malloc(sizeof(char *) * LINK_FIELD_COUNT);
-  if (!(*collection)[current_count]) {
-    *collection = old_collection;
-    *inner_counts = old_inner_counts;
-    free(new_collection);
-    free(new_inner_counts);
-    return current_count;
-  }
-
-  (*collection)[current_count][0] = strdup(href ? href : "");
-  (*collection)[current_count][1] = strdup(label ? label : "");
-  (*collection)[current_count][2] = strdup(class_name ? class_name : "");
-
-  if (!(*collection)[current_count][0] || !(*collection)[current_count][1] ||
-      !(*collection)[current_count][2]) {
-    free((*collection)[current_count][0]);
-    free((*collection)[current_count][1]);
-    free((*collection)[current_count][2]);
-    free((*collection)[current_count]);
-    *collection = old_collection;
-    *inner_counts = old_inner_counts;
-    free(new_collection);
-    free(new_inner_counts);
-    return current_count;
-  }
-
-  (*inner_counts)[current_count] = LINK_FIELD_COUNT;
-
-  free(old_collection);
-  free(old_inner_counts);
-  return current_count + 1;
 }
 
 int build_pagination(int page, char *(*href_builder)(int page, void *data),

src/Utility/Utility.h

@@ -24,6 +24,10 @@ int is_engine_id_enabled(const char *engine_id);
 int get_user_engines(char ***out_ids, int *out_count);
 int user_engines_contains(const char *engine_id, char **ids, int count);
 
+int append_string_row(char ****matrix, int **inner_counts, int row_count,
+                      const char *const *values, int field_count);
+void free_string_matrix(char ***matrix, int *inner_counts, int row_count);
+
 int add_link_to_collection(const char *href, const char *label,
                            const char *class_name, char ****collection,
                            int **inner_counts, int current_count);