[PATCH tabled] server/bucket.c: don't deref NULL upon failed malloc

[PATCH tabled] server/bucket.c: don't deref NULL upon failed malloc

[Jim Meyering]

This was a little more work to fix.
Can't return early without leaking, so I made all
of those P derefs conditional on P being non-NULL.
I factored out the append_const definition to avoid
having to wrap the long lines with the increased indentation.

From b354befe9f7fe754c5fe012e424ccec84ef4e96d Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@redhat.com>
Date: Thu, 23 Sep 2010 12:47:32 +0200
Subject: [PATCH tabled] server/bucket.c: don't deref NULL upon failed malloc


Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 server/bucket.c |   24 +++++++++++++++---------
 1 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/server/bucket.c b/server/bucket.c
index eb03e03..a4af385 100644
--- a/server/bucket.c
+++ b/server/bucket.c
@@ -1,6 +1,6 @@

 /*
- * Copyright 2008-2009 Red Hat, Inc.
+ * Copyright 2008-2010 Red Hat, Inc.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -788,28 +788,34 @@ static GList *bucket_list_pfx(GList *content, GHashTable *common_pfx,
 	s = malloc(cpfx_len);
 	p = s;

+#define append_const(buf, c) \
+  do { memcpy(buf, c, sizeof(c)-1); (buf) += sizeof(c)-1; } while (0)
+
 	tmpl = pfx_list;
 	while (tmpl) {
 		prefix = (char *) tmpl->data;
 		pfx_len = strlen(prefix);

-		memcpy(p, optag, sizeof(optag)-1);  p += sizeof(optag)-1;
-		memcpy(p, pfoptag, sizeof(pfoptag)-1);  p += sizeof(pfoptag)-1;
-		memcpy(p, prefix, pfx_len);  p += pfx_len;
-		memcpy(p, delim, delim_len);  p += delim_len;
-		memcpy(p, pfedtag, sizeof(pfedtag)-1);  p += sizeof(pfedtag)-1;
-		memcpy(p, edtag, sizeof(edtag)-1);  p += sizeof(edtag)-1;
+		if (p) {
+			append_const(p, optag);
+			append_const(p, pfoptag);
+			memcpy(p, prefix, pfx_len);  p += pfx_len;
+			memcpy(p, delim, delim_len);  p += delim_len;
+			append_const(p, pfedtag);
+			append_const(p, edtag);
+		}

 		free(prefix);

 		tmpl = tmpl->next;
 	}
-	*p = 0;
+	if (p)
+		*p = 0;

 	free(delim);
 	g_list_free(pfx_list);

-	return g_list_append(content, s);
+	return s ? g_list_append(content, s) : content;
 }

 struct bucket_list_info {
--
1.7.3.234.g7bba3

Re: [PATCH tabled] server/bucket.c: don't deref NULL upon failed malloc

[Pete Zaitcev]

On Thu, 23 Sep 2010 12:53:06 +0200
Jim Meyering <jim@meyering.net> wrote:

> I factored out the append_const definition to avoid
> having to wrap the long lines with the increased indentation.

>  	p = s;
> 
> +#define append_const(buf, c) \
> +  do { memcpy(buf, c, sizeof(c)-1); (buf) += sizeof(c)-1; } while (0)
> +
>  	tmpl = pfx_list;
>  	while (tmpl) {

Dunno about Jeff, but I would definitely put an #undef to it
after the loop, just in case.

-- Pete

Re: [PATCH tabled] server/bucket.c: don't deref NULL upon failed malloc

[Jeff Garzik]

On 09/23/2010 10:03 AM, Pete Zaitcev wrote:
> On Thu, 23 Sep 2010 12:53:06 +0200
> Jim Meyering<jim@meyering.net>  wrote:
>
>> I factored out the append_const definition to avoid
>> having to wrap the long lines with the increased indentation.
>
>>   	p = s;
>>
>> +#define append_const(buf, c) \
>> +  do { memcpy(buf, c, sizeof(c)-1); (buf) += sizeof(c)-1; } while (0)
>> +
>>   	tmpl = pfx_list;
>>   	while (tmpl) {
>
> Dunno about Jeff, but I would definitely put an #undef to it
> after the loop, just in case.

Yeah, or after the function.