Le 08/04/2021 à 20:59, Tim Duesterhus a écrit :
Willy,
Christopher,

This one comes with dynamic allocation. The next patch will add an optimization
for a small number of arguments. However dynamic allocation within the main
processing logic is pretty ugly, so this should be looked at further.


Dynamic allocation should be avoided here. Definitely. I may propose you an alternative by using the trash buffer area to store the ist array, via a cast. It may be considered a bit ugly at first glance, but if you think about it as a trash memory space, it is not so shocking. We've already used this trick for the HTX and regular buffers. If you do it that way, by default, it gives you 1024 slots. And you may return an alloc failure beyond this value. It seems reasonable :)

+/* Compares two query parameters by name. Query parameters are ordered
+ * as with memcmp. Shorter parameter names are ordered lower. Identical
+ * parameter names are compared by their pointer to maintain a stable
+ * sort.
+ */
+static int query_param_cmp(const void *a, const void *b)
+{
+       const struct ist param_a = *(struct ist*)a;
+       const struct ist param_b = *(struct ist*)b;
+       const struct ist param_a_name = iststop(param_a, '=');
+       const struct ist param_b_name = iststop(param_b, '=');
+
+       int cmp = memcmp(istptr(param_a_name), istptr(param_b_name), 
MIN(istlen(param_a_name), istlen(param_b_name)));
+
+       if (cmp != 0)
+               return cmp;
+
+       if (istlen(param_a_name) < istlen(param_b_name))
+               return -1;
+
+       if (istlen(param_a_name) > istlen(param_b_name))
+               return 1;
+
+       if (istptr(param_a) < istptr(param_b))
+               return -1;
+
+       if (istptr(param_a) > istptr(param_b))
+               return 1;
+
+       return 0;
+}

To make it more simple, you may use istdiff instead.

+
+/* Sorts the parameters within the given query string. Returns an ist 
containing
+ * the new path and backed by `trash` or IST_NULL if the `len` not sufficiently
+ * large to store the resulting path.
+ */
+struct ist uri_normalizer_query_sort(const struct ist query, const char delim, 
char *trash, size_t len)
+{

I hadn't noticed it till now, but please don't use "trash" variable name, it is confusing with the trash buffer used almost everywhere. Especially because of my next comment :)

+       struct ist scanner = istadv(query, 1);
+       struct ist *params = NULL;
+       struct ist newquery = ist2(trash, 0);
+       size_t param_count = 0;
+       size_t i;
+
+       if (len < istlen(query))
+               return IST_NULL;
+
+       while (istlen(scanner) > 0) {
+               const struct ist param = istsplit(&scanner, delim);
+               struct ist *realloc = reallocarray(params, param_count + 1, 
sizeof(*realloc));

Here, instead of a dynamic array, you may use the trash buffer area (declared from outside the loop). For instance:

    struct ist *params = (struct ist *)b_orig(&trash);
    size_t max_param = b_size(&trash) / sizeof(*params);

+
+               if (!realloc)
+                       goto fail;
+
+               params = realloc;
+
+               params[param_count] = param;
+               param_count++;
+       }
+
+       qsort(params, param_count, sizeof(*params), query_param_cmp);
+
+       newquery = __istappend(newquery, '?');
+       for (i = 0; i < param_count; i++) {
+               if (i > 0)
+                       newquery = __istappend(newquery, '&');
+
+               if (istcat(&newquery, params[i], len) < 0)
+                       goto fail;
+       }
+
+       free(params);
+
+       return newquery;
+
+  fail:
+       free(params);
+
+       return IST_NULL;
+}
/*
   * Local variables:



--
Christopher Faulet

Reply via email to