Mailing-List: contact commits-help@avro.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@avro.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1062872 - in /avro/trunk/lang/c: src/datum.c
 tests/test_avro_data.c
Date: Mon, 24 Jan 2011 16:52:09 -0000
To: commits@avro.apache.org
From: brucem@apache.org
Message-Id: <20110124165210.0B65A2388A38@eris.apache.org>

Author: brucem
Date: Mon Jan 24 16:52:09 2011
New Revision: 1062872

URL: http://svn.apache.org/viewvc?rev=1062872&view=rev
Log:
AVRO-745. Fixed memory bug in bytes/string/fixed values.

The avro_bytes_set function (and its string and fixed counterparts) are
supposed to save a copy of the value, which the avro_datum_t instance
takes full control over.  Unfortunately, we were making that copy but
then saving the original pointer, which would lead to a segfault when
freeing the datum.  Whoops...

Modified:
    avro/trunk/lang/c/src/datum.c
    avro/trunk/lang/c/tests/test_avro_data.c

Modified: avro/trunk/lang/c/src/datum.c
URL: http://svn.apache.org/viewvc/avro/trunk/lang/c/src/datum.c?rev=1062872&r1=1062871&r2=1062872&view=diff
==============================================================================
--- avro/trunk/lang/c/src/datum.c (original)
+++ avro/trunk/lang/c/src/datum.c Mon Jan 24 16:52:09 2011
@@ -118,7 +118,8 @@ int avro_string_set(avro_datum_t datum, 
 	if (!string_copy) {
 		return ENOMEM;
 	}
-	rval = avro_string_set_private(datum, p, 0, avro_str_free_wrapper);
+	rval = avro_string_set_private(datum, string_copy, 0,
+				       avro_str_free_wrapper);
 	if (rval) {
 		avro_str_free(string_copy);
 	}
@@ -206,7 +207,7 @@ int avro_bytes_set(avro_datum_t datum, c
 		return ENOMEM;
 	}
 	memcpy(bytes_copy, bytes, size);
-	rval = avro_bytes_set_private(datum, bytes, size, avro_free_wrapper);
+	rval = avro_bytes_set_private(datum, bytes_copy, size, avro_free_wrapper);
 	if (rval) {
 		avro_free(bytes_copy, size);
 	}
@@ -643,7 +644,7 @@ avro_datum_t avro_fixed(const char *name
 		return NULL;
 	}
 	memcpy(bytes_copy, bytes, size);
-	return avro_fixed_private(name, bytes, size, avro_free_wrapper);
+	return avro_fixed_private(name, bytes_copy, size, avro_free_wrapper);
 }
 
 avro_datum_t avro_wrapfixed(const char *name, const char *bytes,
@@ -689,7 +690,7 @@ int avro_fixed_set(avro_datum_t datum, c
 		return ENOMEM;
 	}
 	memcpy(bytes_copy, bytes, size);
-	rval = avro_fixed_set_private(datum, bytes, size, avro_free_wrapper);
+	rval = avro_fixed_set_private(datum, bytes_copy, size, avro_free_wrapper);
 	if (rval) {
 		avro_free(bytes_copy, size);
 	}

Modified: avro/trunk/lang/c/tests/test_avro_data.c
URL: http://svn.apache.org/viewvc/avro/trunk/lang/c/tests/test_avro_data.c?rev=1062872&r1=1062871&r2=1062872&view=diff
==============================================================================
--- avro/trunk/lang/c/tests/test_avro_data.c (original)
+++ avro/trunk/lang/c/tests/test_avro_data.c Mon Jan 24 16:52:09 2011
@@ -162,6 +162,13 @@ static int test_string(void)
 		  "\"Four score and seven years ago\"");
 	avro_datum_decref(datum);
 
+	// The following should bork if we don't copy the string value
+	// correctly (since we'll try to free a static string).
+
+	datum = avro_string("this should be copied");
+	avro_string_set(datum, "also this");
+	avro_datum_decref(datum);
+
 	avro_schema_decref(writer_schema);
 	return 0;
 }
@@ -190,8 +197,15 @@ static int test_bytes(void)
 	}
 	avro_datum_decref(datum);
 	avro_datum_decref(expected_datum);
-	avro_schema_decref(writer_schema);
 
+	// The following should bork if we don't copy the bytes value
+	// correctly (since we'll try to free a static string).
+
+	datum = avro_bytes("original", 8);
+	avro_bytes_set(datum, "alsothis", 8);
+	avro_datum_decref(datum);
+
+	avro_schema_decref(writer_schema);
 	return 0;
 }
 
@@ -510,8 +524,15 @@ static int test_fixed(void)
 	}
 	avro_datum_decref(datum);
 	avro_datum_decref(expected_datum);
-	avro_schema_decref(schema);
 
+	// The following should bork if we don't copy the fixed value
+	// correctly (since we'll try to free a static string).
+
+	datum = avro_fixed("msg", "original", 8);
+	avro_fixed_set(datum, "alsothis", 8);
+	avro_datum_decref(datum);
+
+	avro_schema_decref(schema);
 	return 0;
 }