avro-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dcrea...@apache.org
Subject svn commit: r1242603 - in /avro/trunk: CHANGES.txt lang/c/examples/quickstop.c lang/c/src/avro/schema.h lang/c/src/datafile.c lang/c/src/schema.c lang/c/tests/test_avro_schema.c lang/c/tests/test_avro_values.c
Date Thu, 09 Feb 2012 23:26:33 GMT
Author: dcreager
Date: Thu Feb  9 23:26:32 2012
New Revision: 1242603

URL: http://svn.apache.org/viewvc?rev=1242603&view=rev
Log:
AVRO-980. C: avro_schema_from_json_length

We now explicitly document that the length and error parameters are
unused in the avro_schema_from_json function.  (The error parameter
isn't needed, since any errors parsing or interpreting the JSON text are
available using the avro_strerror() function.)

This patch also adds the new avro_schema_from_json_length function.
This function actually uses its length parameter, and we explicitly
document that the length should *not* include any NUL terminator, if one
is present.  We also provide the avro_schema_from_json_literal helper
macro, which automatically calculates the size of a literal JSON string
at compile time.  (To work, the JSON string must be defined as a char[],
not a char *.)

We decided to fix this bug using a new function because there's existing
code out there that's already assuming that avro_schema_from_json's len
parameter is unused.  (Or at least, they're assuming different things
about what kind of value to pass in.)  This solution ensures that
existing code still works, while providing a new function for the
(needed) non-NUL-terminated case.  It comes at the expense of a sloppy
signature for the existing avro_schema_from_json function...but then,
the signature was already sloppy.  We're at least not adding any *new*
sloppiness.

We also now use the new avro_schema_from_json_length function when
reading an Avro object container file.  This means that we no longer
need to append a NUL terminator to the schema JSON when reading in the
container file's header.  (We don't *need to*, but we still do.  One
change at a time.)

Contributed by Michael Cooper, modified by dcreager

Modified:
    avro/trunk/CHANGES.txt
    avro/trunk/lang/c/examples/quickstop.c
    avro/trunk/lang/c/src/avro/schema.h
    avro/trunk/lang/c/src/datafile.c
    avro/trunk/lang/c/src/schema.c
    avro/trunk/lang/c/tests/test_avro_schema.c
    avro/trunk/lang/c/tests/test_avro_values.c

Modified: avro/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/avro/trunk/CHANGES.txt?rev=1242603&r1=1242602&r2=1242603&view=diff
==============================================================================
--- avro/trunk/CHANGES.txt (original)
+++ avro/trunk/CHANGES.txt Thu Feb  9 23:26:32 2012
@@ -77,6 +77,10 @@ Avro 1.6.2 (13 February 2012)
     AVRO-1021. Clarify some naming issues in the specification.
     (Raymie Stata via cutting)
 
+    AVRO-980. C: avro_schema_from_json ignores length parameter.  Add
+    avro_schema_from_json_length that doesn't.
+    (Michael Cooper and dcreager)
+
   BUG FIXES
 
     AVRO-962. Java: Fix Maven plugin to support string type override.

Modified: avro/trunk/lang/c/examples/quickstop.c
URL: http://svn.apache.org/viewvc/avro/trunk/lang/c/examples/quickstop.c?rev=1242603&r1=1242602&r2=1242603&view=diff
==============================================================================
--- avro/trunk/lang/c/examples/quickstop.c (original)
+++ avro/trunk/lang/c/examples/quickstop.c Thu Feb  9 23:26:32 2012
@@ -25,7 +25,7 @@ avro_schema_t person_schema;
 int64_t id = 0;
 
 /* A simple schema for our tutorial */
-#define PERSON_SCHEMA \
+const char  PERSON_SCHEMA[] =
 "{\"type\":\"record\",\
   \"name\":\"Person\",\
   \"fields\":[\
@@ -33,14 +33,12 @@ int64_t id = 0;
      {\"name\": \"First\", \"type\": \"string\"},\
      {\"name\": \"Last\", \"type\": \"string\"},\
      {\"name\": \"Phone\", \"type\": \"string\"},\
-     {\"name\": \"Age\", \"type\": \"int\"}]}"
+     {\"name\": \"Age\", \"type\": \"int\"}]}";
 
 /* Parse schema into a schema data structure */
 void init_schema(void)
 {
-	avro_schema_error_t error;
-	if (avro_schema_from_json(PERSON_SCHEMA, sizeof(PERSON_SCHEMA),
-				  &person_schema, &error)) {
+	if (avro_schema_from_json_literal(PERSON_SCHEMA, &person_schema)) {
 		fprintf(stderr, "Unable to parse person schema\n");
 		exit(EXIT_FAILURE);
 	}

Modified: avro/trunk/lang/c/src/avro/schema.h
URL: http://svn.apache.org/viewvc/avro/trunk/lang/c/src/avro/schema.h?rev=1242603&r1=1242602&r2=1242603&view=diff
==============================================================================
--- avro/trunk/lang/c/src/avro/schema.h (original)
+++ avro/trunk/lang/c/src/avro/schema.h Thu Feb  9 23:26:32 2012
@@ -83,9 +83,20 @@ avro_schema_t avro_schema_link(avro_sche
 avro_schema_t avro_schema_link_target(avro_schema_t schema);
 
 typedef struct avro_schema_error_t_ *avro_schema_error_t;
-int avro_schema_from_json(const char *jsontext,
-			  const int32_t len,
-			  avro_schema_t * schema, avro_schema_error_t * error);
+
+int avro_schema_from_json(const char *jsontext, int32_t unused1,
+			  avro_schema_t *schema, avro_schema_error_t *unused2);
+
+/* jsontext does not need to be NUL terminated.  length must *NOT*
+ * include the NUL terminator, if one is present. */
+int avro_schema_from_json_length(const char *jsontext, size_t length,
+				 avro_schema_t *schema);
+
+/* A helper macro for loading a schema from a string literal.  The
+ * literal must be declared as a char[], not a char *, since we use the
+ * sizeof operator to determine its length. */
+#define avro_schema_from_json_literal(json, schema) \
+    (avro_schema_from_json_length((json), sizeof((json))-1, (schema)))
 
 int avro_schema_to_specific(avro_schema_t schema, const char *prefix);
 

Modified: avro/trunk/lang/c/src/datafile.c
URL: http://svn.apache.org/viewvc/avro/trunk/lang/c/src/datafile.c?rev=1242603&r1=1242602&r2=1242603&view=diff
==============================================================================
--- avro/trunk/lang/c/src/datafile.c (original)
+++ avro/trunk/lang/c/src/datafile.c Thu Feb  9 23:26:32 2012
@@ -213,7 +213,6 @@ static int file_read_header(avro_reader_
 	avro_value_t schema_bytes;
 	const void *p;
 	size_t len;
-	avro_schema_error_t schema_error;
 
 	check(rval, avro_read(reader, magic, sizeof(magic)));
 	if (magic[0] != 'O' || magic[1] != 'b' || magic[2] != 'j'
@@ -273,7 +272,7 @@ static int file_read_header(avro_reader_
 	}
 
 	avro_value_get_bytes(&schema_bytes, &p, &len);
-	rval = avro_schema_from_json(p, len, writers_schema, &schema_error);
+	rval = avro_schema_from_json_length(p, len, writers_schema);
 	if (rval) {
 		avro_prefix_error("Cannot parse file header: ");
 		avro_value_decref(&meta);

Modified: avro/trunk/lang/c/src/schema.c
URL: http://svn.apache.org/viewvc/avro/trunk/lang/c/src/schema.c?rev=1242603&r1=1242602&r2=1242603&view=diff
==============================================================================
--- avro/trunk/lang/c/src/schema.c (original)
+++ avro/trunk/lang/c/src/schema.c Thu Feb  9 23:26:32 2012
@@ -34,11 +34,6 @@
 
 #define DEFAULT_TABLE_SIZE 32
 
-struct avro_schema_error_t_ {
-	st_table *named_schemas;
-	json_error_t json_error;
-};
-
 static void avro_schema_init(avro_schema_t schema, avro_type_t type)
 {
 	schema->type = type;
@@ -658,29 +653,6 @@ avro_schema_t avro_schema_record_field_g
 	return val.field->type;
 }
 
-static int
-save_named_schemas(const char *name, avro_schema_t schema,
-		   avro_schema_error_t * error)
-{
-	st_table *st = (*error)->named_schemas;
-	return st_insert(st, (st_data_t) name, (st_data_t) schema);
-}
-
-static avro_schema_t
-find_named_schemas(const char *name, avro_schema_error_t * error)
-{
-	st_table *st = (*error)->named_schemas;
-	union {
-		avro_schema_t schema;
-		st_data_t data;
-	} val;
-	if (st_lookup(st, (st_data_t) name, &(val.data))) {
-		return val.schema;
-	}
-	avro_set_error("No schema type named %s", name);
-	return NULL;
-};
-
 avro_schema_t avro_schema_link(avro_schema_t to)
 {
 	if (!is_avro_named_type(to)) {
@@ -708,8 +680,28 @@ avro_schema_t avro_schema_link_target(av
 }
 
 static int
-avro_type_from_json_t(json_t * json, avro_type_t * type,
-		      avro_schema_error_t * error, avro_schema_t * named_type)
+save_named_schemas(const char *name, avro_schema_t schema, st_table *st)
+{
+	return st_insert(st, (st_data_t) name, (st_data_t) schema);
+}
+
+static avro_schema_t
+find_named_schemas(const char *name, st_table *st)
+{
+	union {
+		avro_schema_t schema;
+		st_data_t data;
+	} val;
+	if (st_lookup(st, (st_data_t) name, &(val.data))) {
+		return val.schema;
+	}
+	avro_set_error("No schema type named %s", name);
+	return NULL;
+};
+
+static int
+avro_type_from_json_t(json_t *json, avro_type_t *type,
+		      st_table *named_schemas, avro_schema_t *named_type)
 {
 	json_t *json_type;
 	const char *type_str;
@@ -760,7 +752,7 @@ avro_type_from_json_t(json_t * json, avr
 		*type = AVRO_MAP;
 	} else if (strcmp(type_str, "fixed") == 0) {
 		*type = AVRO_FIXED;
-	} else if ((*named_type = find_named_schemas(type_str, error))) {
+	} else if ((*named_type = find_named_schemas(type_str, named_schemas))) {
 		*type = AVRO_LINK;
 	} else {
 		avro_set_error("Unknown Avro \"type\": %s", type_str);
@@ -770,20 +762,20 @@ avro_type_from_json_t(json_t * json, avr
 }
 
 static int
-avro_schema_from_json_t(json_t * json, avro_schema_t * schema,
-			avro_schema_error_t * error)
+avro_schema_from_json_t(json_t *json, avro_schema_t *schema,
+			st_table *named_schemas)
 {
 	avro_type_t type = 0;
 	unsigned int i;
-	avro_schema_t named_schemas = NULL;
+	avro_schema_t named_type = NULL;
 
-	if (avro_type_from_json_t(json, &type, error, &named_schemas)) {
+	if (avro_type_from_json_t(json, &type, named_schemas, &named_type)) {
 		return EINVAL;
 	}
 
 	switch (type) {
 	case AVRO_LINK:
-		*schema = avro_schema_link(named_schemas);
+		*schema = avro_schema_link(named_type);
 		break;
 
 	case AVRO_STRING:
@@ -854,7 +846,7 @@ avro_schema_from_json_t(json_t * json, a
 			}
 			*schema =
 			    avro_schema_record(record_name, record_namespace);
-			if (save_named_schemas(record_name, *schema, error)) {
+			if (save_named_schemas(record_name, *schema, named_schemas)) {
 				avro_set_error("Cannot save record schema");
 				return ENOMEM;
 			}
@@ -888,7 +880,7 @@ avro_schema_from_json_t(json_t * json, a
 				field_rval =
 				    avro_schema_from_json_t(json_field_type,
 							    &json_field_type_schema,
-							    error);
+							    named_schemas);
 				if (field_rval) {
 					avro_schema_decref(*schema);
 					return field_rval;
@@ -934,7 +926,7 @@ avro_schema_from_json_t(json_t * json, a
 				return EINVAL;
 			}
 			*schema = avro_schema_enum(name);
-			if (save_named_schemas(name, *schema, error)) {
+			if (save_named_schemas(name, *schema, named_schemas)) {
 				avro_set_error("Cannot save enum schema");
 				return ENOMEM;
 			}
@@ -971,7 +963,7 @@ avro_schema_from_json_t(json_t * json, a
 			}
 			items_rval =
 			    avro_schema_from_json_t(json_items, &items_schema,
-						    error);
+						    named_schemas);
 			if (items_rval) {
 				return items_rval;
 			}
@@ -992,7 +984,7 @@ avro_schema_from_json_t(json_t * json, a
 			}
 			values_rval =
 			    avro_schema_from_json_t(json_values, &values_schema,
-						    error);
+						    named_schemas);
 			if (values_rval) {
 				return values_rval;
 			}
@@ -1019,7 +1011,7 @@ avro_schema_from_json_t(json_t * json, a
 				}
 				schema_rval =
 				    avro_schema_from_json_t(schema_json, &s,
-							    error);
+							    named_schemas);
 				if (schema_rval != 0) {
 					avro_schema_decref(*schema);
 					return schema_rval;
@@ -1052,7 +1044,7 @@ avro_schema_from_json_t(json_t * json, a
 			size = json_integer_value(json_size);
 			name = json_string_value(json_name);
 			*schema = avro_schema_fixed(name, size);
-			if (save_named_schemas(name, *schema, error)) {
+			if (save_named_schemas(name, *schema, named_schemas)) {
 				avro_set_error("Cannot save fixed schema");
 				return ENOMEM;
 			}
@@ -1066,52 +1058,65 @@ avro_schema_from_json_t(json_t * json, a
 	return 0;
 }
 
+static int
+avro_schema_from_json_root(json_t *root, avro_schema_t *schema)
+{
+	int  rval;
+	st_table *named_schemas;
+
+	named_schemas = st_init_strtable_with_size(DEFAULT_TABLE_SIZE);
+	if (!named_schemas) {
+		avro_set_error("Cannot allocate named schema map");
+		json_decref(root);
+		return ENOMEM;
+	}
+
+	/* json_dumpf(root, stderr, 0); */
+	rval = avro_schema_from_json_t(root, schema, named_schemas);
+	json_decref(root);
+	st_free_table(named_schemas);
+	return rval;
+}
+
 int
 avro_schema_from_json(const char *jsontext, const int32_t len,
-		      avro_schema_t * schema, avro_schema_error_t * e)
+		      avro_schema_t *schema, avro_schema_error_t *e)
 {
 	check_param(EINVAL, jsontext, "JSON text");
 	check_param(EINVAL, schema, "schema pointer");
 
-	json_t *root;
-	int rval = 0;
-	avro_schema_error_t error;
+	json_t  *root;
+	json_error_t  json_error;
 
 	AVRO_UNUSED(len);
+	AVRO_UNUSED(e);
 
-	error = avro_new(struct avro_schema_error_t_);
-	if (!error) {
-		avro_set_error("Cannot allocate schema error buffer");
-		return ENOMEM;
+	root = json_loads(jsontext, 0, &json_error);
+	if (!root) {
+		avro_set_error("Error parsing JSON: %s", json_error.text);
+		return EINVAL;
 	}
-	*e = error;
 
-	error->named_schemas = st_init_strtable_with_size(DEFAULT_TABLE_SIZE);
-	if (!error->named_schemas) {
-		avro_set_error("Cannot allocate named schema map");
-		avro_freet(struct avro_schema_error_t_, error);
-		return ENOMEM;
-	}
+	return avro_schema_from_json_root(root, schema);
+}
 
-	root = json_loads(jsontext, 0, &error->json_error);
+int
+avro_schema_from_json_length(const char *jsontext, size_t length,
+			     avro_schema_t *schema)
+{
+	check_param(EINVAL, jsontext, "JSON text");
+	check_param(EINVAL, schema, "schema pointer");
+
+	json_t  *root;
+	json_error_t  json_error;
+
+	root = json_loadb(jsontext, length, 0, &json_error);
 	if (!root) {
-		avro_set_error("Error parsing JSON: %s", error->json_error.text);
-		st_free_table(error->named_schemas);
-		avro_freet(struct avro_schema_error_t_, error);
+		avro_set_error("Error parsing JSON: %s", json_error.text);
 		return EINVAL;
 	}
 
-	/*
-	 * json_dumpf(root, stderr, 0); 
-	 */
-	rval = avro_schema_from_json_t(root, schema, e);
-	json_decref(root);
-	st_free_table(error->named_schemas);
-	if (rval == 0) {
-		/* no need for an error return */
-		avro_freet(struct avro_schema_error_t_, error);
-	}
-	return rval;
+	return avro_schema_from_json_root(root, schema);
 }
 
 avro_schema_t avro_schema_copy(avro_schema_t schema)

Modified: avro/trunk/lang/c/tests/test_avro_schema.c
URL: http://svn.apache.org/viewvc/avro/trunk/lang/c/tests/test_avro_schema.c?rev=1242603&r1=1242602&r2=1242603&view=diff
==============================================================================
--- avro/trunk/lang/c/tests/test_avro_schema.c (original)
+++ avro/trunk/lang/c/tests/test_avro_schema.c Thu Feb  9 23:26:32 2012
@@ -35,7 +35,6 @@ static void run_tests(char *dirpath, int
 	struct dirent *dent;
 	FILE *fp;
 	avro_schema_t schema;
-	avro_schema_error_t avro_schema_error;
 
 	dir = opendir(dirpath);
 	if (dir == NULL) {
@@ -58,8 +57,7 @@ static void run_tests(char *dirpath, int
 			rval = fread(jsontext, 1, sizeof(jsontext) - 1, fp);
 			jsontext[rval] = '\0';
 			test_rval =
-			    avro_schema_from_json(jsontext, jsonlen, &schema,
-						  &avro_schema_error);
+			    avro_schema_from_json(jsontext, 0, &schema, NULL);
 			test_cases++;
 			if (test_rval == 0) {
 				if (should_pass) {

Modified: avro/trunk/lang/c/tests/test_avro_values.c
URL: http://svn.apache.org/viewvc/avro/trunk/lang/c/tests/test_avro_values.c?rev=1242603&r1=1242602&r2=1242603&view=diff
==============================================================================
--- avro/trunk/lang/c/tests/test_avro_values.c (original)
+++ avro/trunk/lang/c/tests/test_avro_values.c Thu Feb  9 23:26:32 2012
@@ -916,7 +916,7 @@ test_array(void)
 static int
 test_enum(void)
 {
-	static const char  *SCHEMA_JSON =
+	static const char  SCHEMA_JSON[] =
 	"{"
 	"  \"type\": \"enum\","
 	"  \"name\": \"suits\","
@@ -924,9 +924,7 @@ test_enum(void)
 	"}";
 
 	avro_schema_t  enum_schema = NULL;
-	avro_schema_error_t  err;
-	if (avro_schema_from_json(SCHEMA_JSON, sizeof(SCHEMA_JSON),
-				  &enum_schema, &err)) {
+	if (avro_schema_from_json_literal(SCHEMA_JSON, &enum_schema)) {
 		fprintf(stderr, "Error parsing schema:\n  %s\n",
 			avro_strerror());
 		return EXIT_FAILURE;
@@ -974,7 +972,7 @@ test_enum(void)
 static int
 test_fixed(void)
 {
-	static const char  *SCHEMA_JSON =
+	static const char  SCHEMA_JSON[] =
 	"{"
 	"  \"type\": \"fixed\","
 	"  \"name\": \"ipv4\","
@@ -982,9 +980,7 @@ test_fixed(void)
 	"}";
 
 	avro_schema_t  fixed_schema = NULL;
-	avro_schema_error_t  err;
-	if (avro_schema_from_json(SCHEMA_JSON, sizeof(SCHEMA_JSON),
-				  &fixed_schema, &err)) {
+	if (avro_schema_from_json_literal(SCHEMA_JSON, &fixed_schema)) {
 		fprintf(stderr, "Error parsing schema:\n  %s\n",
 			avro_strerror());
 		return EXIT_FAILURE;
@@ -1198,7 +1194,7 @@ test_map(void)
 static int
 test_record(void)
 {
-	static const char  *SCHEMA_JSON =
+	static const char  SCHEMA_JSON[] =
 	"{"
 	"  \"type\": \"record\","
 	"  \"name\": \"test\","
@@ -1223,9 +1219,7 @@ test_record(void)
 	"}";
 
 	avro_schema_t  record_schema = NULL;
-	avro_schema_error_t  err;
-	if (avro_schema_from_json(SCHEMA_JSON, sizeof(SCHEMA_JSON),
-				  &record_schema, &err)) {
+	if (avro_schema_from_json_literal(SCHEMA_JSON, &record_schema)) {
 		fprintf(stderr, "Error parsing schema:\n  %s\n",
 			avro_strerror());
 		return EXIT_FAILURE;
@@ -1347,7 +1341,7 @@ test_record(void)
 static int
 test_union(void)
 {
-	static const char  *SCHEMA_JSON =
+	static const char  SCHEMA_JSON[] =
 	"["
 	"  \"null\","
 	"  \"int\","
@@ -1356,9 +1350,7 @@ test_union(void)
 	"]";
 
 	avro_schema_t  union_schema = NULL;
-	avro_schema_error_t  err;
-	if (avro_schema_from_json(SCHEMA_JSON, sizeof(SCHEMA_JSON),
-				  &union_schema, &err)) {
+	if (avro_schema_from_json_literal(SCHEMA_JSON, &union_schema)) {
 		fprintf(stderr, "Error parsing schema:\n  %s\n",
 			avro_strerror());
 		return EXIT_FAILURE;



Mime
View raw message