AVRO-3961: [C] Add AVRO_INVALID to avro_type_t (#2799)

EINVAL was already being cast to an avro_type_t in the public interface, and
making AVRO_INVALID explicit in the enum removes two preexisting workarounds.

AVRO_INVALID uses the same value as EINVAL for ABI compatibility, and a test is
added to ensure no collisions with the preexisting enums.

Signed-off-by: Sahil Kang <[email protected]>
Signed-off-by: Sahil Kang <[email protected]>

lang/c/docs/index.txt

@@ -117,7 +117,7 @@ This section provides an overview of the methods that you can call on an
 interface, but not all of them make sense for all Avro schema types.
 For instance, you won't be able to call +avro_value_set_boolean+ on an
 Avro array value.  If you try to call an inappropriate method, we'll
-return an +EINVAL+ error code.
+return an +EINVAL+/+AVRO_INVALID+ error code.
 
 Note that the functions in this section apply to _all_ Avro values,
 regardless of which value implementation is used under the covers.  This

lang/c/src/avro/basics.h

@@ -24,6 +24,7 @@ extern "C" {
 #define CLOSE_EXTERN
 #endif
 
+#include <errno.h>
 
 enum avro_type_t {
 	AVRO_STRING,
@@ -40,7 +41,8 @@ enum avro_type_t {
 	AVRO_MAP,
 	AVRO_ARRAY,
 	AVRO_UNION,
-	AVRO_LINK
+	AVRO_LINK,
+	AVRO_INVALID = EINVAL,
 };
 typedef enum avro_type_t avro_type_t;
 

lang/c/src/consume-binary.c

@@ -322,6 +322,9 @@ avro_consume_binary(avro_reader_t reader, avro_consumer_t *consumer, void *ud)
 	case AVRO_LINK:
 		avro_set_error("Consumer can't consume a link schema directly");
 		return EINVAL;
+	case AVRO_INVALID:
+		avro_set_error("Consumer can't consume an invalid schema");
+		return EINVAL;
 	}
 
 	return 0;

lang/c/src/datum.c

@@ -1086,6 +1086,7 @@ static void avro_datum_free(avro_datum_t datum)
 			}
 			break;
 		case AVRO_NULL:
+		case AVRO_INVALID:
 			/* Nothing allocated */
 			break;
 

lang/c/src/datum_equal.c

@@ -181,6 +181,12 @@ int avro_datum_equal(const avro_datum_t a, const avro_datum_t b)
 		 * TODO 
 		 */
 		return 0;
+	case AVRO_INVALID:
+		/*
+		 * Invalid datums should not be compared and returning 0
+		 * matches the other error conditions
+		 */
+		return 0;
 	}
 	return 0;
 }

lang/c/src/datum_size.c

@@ -271,6 +271,7 @@ static int64_t size_datum(avro_writer_t writer, const avro_encoding_t * enc,
 				  avro_datum_to_union(datum));
 
 	case AVRO_LINK:
+	case AVRO_INVALID:
 		break;
 	}
 

lang/c/src/datum_skip.c

@@ -196,6 +196,9 @@ int avro_skip_data(avro_reader_t reader, avro_schema_t writers_schema)
 		    avro_skip_data(reader,
 				   (avro_schema_to_link(writers_schema))->to);
 		break;
+	case AVRO_INVALID:
+		rval = EINVAL;
+		break;
 	}
 
 	return rval;

lang/c/src/datum_validate.c

@@ -188,6 +188,8 @@ avro_schema_datum_validate(avro_schema_t expected_schema, avro_datum_t datum)
 						       datum);
 		}
 		break;
+	case AVRO_INVALID:
+		return EINVAL;
 	}
 	return 0;
 }

lang/c/src/datum_value.c

@@ -80,19 +80,7 @@ avro_datum_value_get_type(const avro_value_iface_t *iface, const void *vself)
 {
 	AVRO_UNUSED(iface);
 	const avro_datum_t  self = (const avro_datum_t) vself;
-#ifdef _WIN32
-#pragma message("#warning: Bug: EINVAL is not of type avro_type_t.")
-#else
-#warning "Bug: EINVAL is not of type avro_type_t."
-#endif
-        /* We shouldn't use EINVAL as the return value to
-         * check_param(), because EINVAL (= 22) is not a valid enum
-         * avro_type_t. This is a structural issue -- we would need a
-         * different interface on all the get_type functions to fix
-         * this. For now, suppressing the error by casting EINVAL to
-         * (avro_type_t) so the code compiles under C++.
-         */
-	check_param((avro_type_t) EINVAL, self, "datum instance");
+	check_param(AVRO_INVALID, self, "datum instance");
 	return avro_typeof(self);
 }
 

lang/c/src/schema.c

@@ -126,6 +126,7 @@ static void avro_schema_free(avro_schema_t schema)
 		case AVRO_DOUBLE:
 		case AVRO_BOOLEAN:
 		case AVRO_NULL:
+		case AVRO_INVALID:
 			/* no memory allocated for primitives */
 			return;
 
@@ -876,15 +877,7 @@ static int
 avro_schema_from_json_t(json_t *json, avro_schema_t *schema,
 			st_table *named_schemas, const char *parent_namespace)
 {
-#ifdef _WIN32
- #pragma message("#warning: Bug: '0' is not of type avro_type_t.")
-#else
- #warning "Bug: '0' is not of type avro_type_t."
-#endif
-  /* We should really have an "AVRO_INVALID" type in
-   * avro_type_t. Suppress warning below in which we set type to 0.
-   */
-	avro_type_t type = (avro_type_t) 0;
+	avro_type_t type = AVRO_INVALID;
 	unsigned int i;
 	avro_schema_t named_type = NULL;
 
@@ -1882,6 +1875,8 @@ avro_schema_to_json2(const avro_schema_t schema, avro_writer_t out,
 		return write_union(out, avro_schema_to_union(schema), parent_namespace);
 	case AVRO_LINK:
 		return write_link(out, avro_schema_to_link(schema), parent_namespace);
+	case AVRO_INVALID:
+		return EINVAL;
 	}
 
 	if (is_avro_primitive(schema)) {

lang/c/tests/CMakeLists.txt

@@ -66,6 +66,7 @@ add_avro_test_checkmem(test_data_structures)
 add_avro_test_checkmem(test_avro_schema)
 add_avro_test_checkmem(test_avro_commons_schema)
 add_avro_test_checkmem(test_avro_schema_names)
+add_avro_test_checkmem(test_avro_type_collision)
 add_avro_test_checkmem(test_avro_values)
 add_avro_test_checkmem(test_avro_766)
 add_avro_test_checkmem(test_avro_968)

lang/c/tests/test_avro_type_collision.c

@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+ * implied.  See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+#include "avro.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#if defined(__GNUC__) || defined(__clang__)
+#pragma GCC diagnostic error "-Wswitch"
+#endif
+
+#define ASSERT_NOT_AVRO_INVALID(type)                                   \
+	if (type == AVRO_INVALID) {                                     \
+		fprintf(stderr, #type " collides with AVRO_INVALID\n"); \
+		exit(EXIT_FAILURE);                                     \
+	} else {                                                        \
+		break;                                                  \
+	}
+
+#define CASE_ASSERTION(type) case type: ASSERT_NOT_AVRO_INVALID(type)
+
+int main(void)
+{
+	avro_schema_t null_schema = avro_schema_null();
+	avro_type_t type = avro_typeof(null_schema);
+	avro_schema_decref(null_schema);
+
+	switch (type) {
+	CASE_ASSERTION(AVRO_STRING)
+	CASE_ASSERTION(AVRO_BYTES)
+	CASE_ASSERTION(AVRO_INT32)
+	CASE_ASSERTION(AVRO_INT64)
+	CASE_ASSERTION(AVRO_FLOAT)
+	CASE_ASSERTION(AVRO_DOUBLE)
+	CASE_ASSERTION(AVRO_BOOLEAN)
+	CASE_ASSERTION(AVRO_NULL)
+	CASE_ASSERTION(AVRO_RECORD)
+	CASE_ASSERTION(AVRO_ENUM)
+	CASE_ASSERTION(AVRO_FIXED)
+	CASE_ASSERTION(AVRO_MAP)
+	CASE_ASSERTION(AVRO_ARRAY)
+	CASE_ASSERTION(AVRO_UNION)
+	CASE_ASSERTION(AVRO_LINK)
+	case AVRO_INVALID:
+		break;
+	}
+
+	return EXIT_SUCCESS;
+}

lang/c/version.sh

@@ -34,9 +34,9 @@
 #         libavro_binary_age = 0
 #         libavro_interface_age = 0
 #
-libavro_micro_version=23
+libavro_micro_version=24
 libavro_interface_age=0
-libavro_binary_age=0
+libavro_binary_age=1
 
 # IGNORE EVERYTHING ELSE FROM HERE DOWN.........
 if test $# != 1; then