You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by dc...@apache.org on 2012/02/10 00:26:33 UTC
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
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;