[hail patch 1/1] Fix calling convention of huri_field_escape

[hail patch 1/1] Fix calling convention of huri_field_escape

[Pete Zaitcev]

Premature optimization is the root of all evil.

Use a sensible convention of not screwing with the argument, at the expense
of extra strdup.

Fortunately, all users are confined to Hail itself, even if huri_field_escape
is exported.

Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>

---
 include/hstor.h |    2 +-
 lib/hstor.c     |   44 +++++++++++++++++++++++++++++---------------
 lib/huri.c      |   10 +++++-----
 3 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/include/hstor.h b/include/hstor.h
index cd04c56..8620d3b 100644
--- a/include/hstor.h
+++ b/include/hstor.h
@@ -144,7 +144,7 @@ extern int hreq_acl_canned(struct http_req *req);
 /* uri.c */
 extern struct http_uri *huri_parse(struct http_uri *uri_dest, char *uri_src_text);
 extern int huri_field_unescape(char *s, int s_len);
-extern char* huri_field_escape (char *signed_str, unsigned char mask);
+extern char* huri_field_escape(const char *signed_str, unsigned char mask);
 
 static inline bool hreq_http11(struct http_req *req)
 {
diff --git a/lib/hstor.c b/lib/hstor.c
index 79e0420..d0d87c7 100644
--- a/lib/hstor.c
+++ b/lib/hstor.c
@@ -383,14 +383,16 @@ bool hstor_get(struct hstor_client *hstor, const char *bucket, const char *key,
 {
 	struct http_req req;
 	char datestr[80], timestr[64], hmac[64], auth[128];
-	char *host, *url, *orig_path;
+	char *host, *url, *unesc_path, *orig_path;
 	struct curl_slist *headers = NULL;
 	int rc;
 
-	if (asprintf(&orig_path, "/%s/%s", bucket, key) < 0)
+	if (asprintf(&unesc_path, "/%s/%s", bucket, key) < 0)
 		goto err_spath;
 
-	orig_path = huri_field_escape(orig_path, PATH_ESCAPE_MASK);
+	orig_path = huri_field_escape(unesc_path, PATH_ESCAPE_MASK);
+	if (!orig_path)
+		goto err_epath;
 
 	memset(&req, 0, sizeof(req));
 	req.method = "GET";
@@ -431,6 +433,7 @@ bool hstor_get(struct hstor_client *hstor, const char *bucket, const char *key,
 	free(url);
 	free(host);
 	free(orig_path);
+	free(unesc_path);
 
 	return (rc == 0);
 
@@ -438,6 +441,8 @@ err_url:
 	free(host);
 err_host:
 	free(orig_path);
+err_epath:
+	free(unesc_path);
 err_spath:
 	return false;
 }
@@ -474,15 +479,17 @@ bool hstor_put(struct hstor_client *hstor, const char *bucket, const char *key,
 {
 	struct http_req req;
 	char datestr[80], timestr[64], hmac[64], auth[128];
-	char *host, *url, *orig_path;
+	char *host, *url, *unesc_path, *orig_path;
 	char *uhdr_buf = NULL;
 	struct curl_slist *headers = NULL;
 	int rc = -1;
 
-	if (asprintf(&orig_path, "/%s/%s", bucket, key) < 0)
+	if (asprintf(&unesc_path, "/%s/%s", bucket, key) < 0)
 		goto err_spath;
 
-	orig_path = huri_field_escape(orig_path, PATH_ESCAPE_MASK);
+	orig_path = huri_field_escape(unesc_path, PATH_ESCAPE_MASK);
+	if (!orig_path)
+		goto err_epath;
 
 	memset(&req, 0, sizeof(req));
 	req.method = "PUT";
@@ -570,6 +577,7 @@ bool hstor_put(struct hstor_client *hstor, const char *bucket, const char *key,
 	free(url);
 	free(host);
 	free(orig_path);
+	free(unesc_path);
 	free(uhdr_buf);
 	return (rc == 0);
 
@@ -579,6 +587,8 @@ err_host:
 	free(uhdr_buf);
 err_ubuf:
 	free(orig_path);
+err_epath:
+	free(unesc_path);
 err_spath:
 	return false;
 }
@@ -616,14 +626,16 @@ bool hstor_del(struct hstor_client *hstor, const char *bucket, const char *key)
 {
 	struct http_req req;
 	char datestr[80], timestr[64], hmac[64], auth[128];
-	char *host, *url, *orig_path;
+	char *host, *url, *unesc_path, *orig_path;
 	struct curl_slist *headers = NULL;
 	int rc;
 
-	if (asprintf(&orig_path, "/%s/%s", bucket, key) < 0)
+	if (asprintf(&unesc_path, "/%s/%s", bucket, key) < 0)
 		goto err_spath;
 
-	orig_path = huri_field_escape(orig_path, PATH_ESCAPE_MASK);
+	orig_path = huri_field_escape(unesc_path, PATH_ESCAPE_MASK);
+	if (!orig_path)
+		goto err_epath;
 
 	memset(&req, 0, sizeof(req));
 	req.method = "DELETE";
@@ -661,6 +673,7 @@ bool hstor_del(struct hstor_client *hstor, const char *bucket, const char *key)
 	free(url);
 	free(host);
 	free(orig_path);
+	free(unesc_path);
 
 	return (rc == 0);
 
@@ -668,6 +681,8 @@ err_url:
 	free(host);
 err_host:
 	free(orig_path);
+err_epath:
+	free(unesc_path);
 err_spath:
 	return false;
 }
@@ -676,7 +691,6 @@ static GString *append_qparam(GString *str, const char *key, const char *val,
 		       char *arg_char)
 {
 	char *stmp;
-	char *v;
 
 	str = g_string_append(str, arg_char);
 	arg_char[0] = '&';
@@ -684,11 +698,11 @@ static GString *append_qparam(GString *str, const char *key, const char *val,
 	str = g_string_append(str, key);
 	str = g_string_append(str, "=");
 
-	v = strdup(val);
-	stmp = huri_field_escape(v, QUERY_ESCAPE_MASK);
-	str = g_string_append(str, stmp);
-	free(stmp);
-	free(v);
+	stmp = huri_field_escape(val, QUERY_ESCAPE_MASK);
+	if (stmp) {
+		str = g_string_append(str, stmp);
+		free(stmp);
+	}
 
 	return str;
 }
diff --git a/lib/huri.c b/lib/huri.c
index 7198536..35d857e 100644
--- a/lib/huri.c
+++ b/lib/huri.c
@@ -25,7 +25,6 @@
 #include <string.h>
 #include <ctype.h>
 #include <stdbool.h>
-#include <glib.h>
 #include <hstor.h>
 
 /* our own ISSPACE.  ANSI isspace is locale dependent */
@@ -221,7 +220,7 @@ static const guchar neednt_escape_table[] =
 	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
 };
 
-char* huri_field_escape (char *signed_str, unsigned char mask)
+char* huri_field_escape(const char *signed_str, unsigned char mask)
 {
   int len;
   int i;
@@ -250,10 +249,12 @@ char* huri_field_escape (char *signed_str, unsigned char mask)
 
   /* Don't escape if unnecessary */
   if (must_escape == FALSE)
-    return signed_str;
+    return strdup(signed_str);
 
   /* Allocate buffer */
-  dst = (gchar*) g_malloc(len + 1);
+  dst = malloc(len + 1);
+  if (!dst)
+    return NULL;
 
   /* Copy */
   for (i = j = 0; str[i]; i++, j++)
@@ -284,7 +285,6 @@ char* huri_field_escape (char *signed_str, unsigned char mask)
     }
   dst[j] = '\0';
 
-  g_free (signed_str);
   return dst;
 }
 

Re: [hail patch 1/1] Fix calling convention of huri_field_escape

[Jeff Garzik]

On 09/27/2010 08:49 PM, Pete Zaitcev wrote:
> Premature optimization is the root of all evil.
>
> Use a sensible convention of not screwing with the argument, at the expense
> of extra strdup.
>
> Fortunately, all users are confined to Hail itself, even if huri_field_escape
> is exported.
>
> Signed-off-by: Pete Zaitcev<zaitcev@redhat.com>
>
> ---
>   include/hstor.h |    2 +-
>   lib/hstor.c     |   44 +++++++++++++++++++++++++++++---------------
>   lib/huri.c      |   10 +++++-----
>   3 files changed, 35 insertions(+), 21 deletions(-)

applied