This is an automated email from the ASF dual-hosted git repository.

mgrigorov pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/avro.git


The following commit(s) were added to refs/heads/main by this push:
     new 9b195c131 AVRO-3961: [C] Add AVRO_INVALID to avro_type_t (#2799)
9b195c131 is described below

commit 9b195c13152427e6c6dab7da112c4634374208bf
Author: Sahil Kang <[email protected]>
AuthorDate: Tue Mar 12 04:32:07 2024 -0700

    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                   |  2 +-
 lang/c/src/avro/basics.h                |  4 ++-
 lang/c/src/consume-binary.c             |  3 ++
 lang/c/src/datum.c                      |  1 +
 lang/c/src/datum_equal.c                |  6 ++++
 lang/c/src/datum_size.c                 |  1 +
 lang/c/src/datum_skip.c                 |  3 ++
 lang/c/src/datum_validate.c             |  2 ++
 lang/c/src/datum_value.c                | 14 +-------
 lang/c/src/schema.c                     | 13 +++----
 lang/c/tests/CMakeLists.txt             |  1 +
 lang/c/tests/test_avro_type_collision.c | 64 +++++++++++++++++++++++++++++++++
 lang/c/version.sh                       |  4 +--
 13 files changed, 92 insertions(+), 26 deletions(-)

diff --git a/lang/c/docs/index.txt b/lang/c/docs/index.txt
index df16f9d96..86f67bc8f 100644
--- a/lang/c/docs/index.txt
+++ b/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
diff --git a/lang/c/src/avro/basics.h b/lang/c/src/avro/basics.h
index 368509b90..62c899c69 100644
--- a/lang/c/src/avro/basics.h
+++ b/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;
 
diff --git a/lang/c/src/consume-binary.c b/lang/c/src/consume-binary.c
index 9f92799d8..5e1db2068 100644
--- a/lang/c/src/consume-binary.c
+++ b/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;
diff --git a/lang/c/src/datum.c b/lang/c/src/datum.c
index 2c4278090..53dfa5ca0 100644
--- a/lang/c/src/datum.c
+++ b/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;
 
diff --git a/lang/c/src/datum_equal.c b/lang/c/src/datum_equal.c
index 2ef750f9b..3875bea04 100644
--- a/lang/c/src/datum_equal.c
+++ b/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;
 }
diff --git a/lang/c/src/datum_size.c b/lang/c/src/datum_size.c
index 770cb655f..be9b98004 100644
--- a/lang/c/src/datum_size.c
+++ b/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;
        }
 
diff --git a/lang/c/src/datum_skip.c b/lang/c/src/datum_skip.c
index aa51d7934..e0ce56164 100644
--- a/lang/c/src/datum_skip.c
+++ b/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;
diff --git a/lang/c/src/datum_validate.c b/lang/c/src/datum_validate.c
index d15ebddda..6167bd63f 100644
--- a/lang/c/src/datum_validate.c
+++ b/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;
 }
diff --git a/lang/c/src/datum_value.c b/lang/c/src/datum_value.c
index a4fa55a0c..597d38c45 100644
--- a/lang/c/src/datum_value.c
+++ b/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);
 }
 
diff --git a/lang/c/src/schema.c b/lang/c/src/schema.c
index 7b389002b..2acad51a1 100644
--- a/lang/c/src/schema.c
+++ b/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)) {
diff --git a/lang/c/tests/CMakeLists.txt b/lang/c/tests/CMakeLists.txt
index 1413a3f37..320016477 100644
--- a/lang/c/tests/CMakeLists.txt
+++ b/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)
diff --git a/lang/c/tests/test_avro_type_collision.c 
b/lang/c/tests/test_avro_type_collision.c
new file mode 100644
index 000000000..1dda590fd
--- /dev/null
+++ b/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;
+}
diff --git a/lang/c/version.sh b/lang/c/version.sh
index be90c0f63..0481bcc23 100755
--- a/lang/c/version.sh
+++ b/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

Reply via email to