[ovs-dev,mointor2,2/9] lib: avoid set size check when generating diff datum from json
diff mbox

Message ID 1445489131-21483-2-git-send-email-azhou@nicira.com
State Changes Requested
Headers show

Commit Message

Andy Zhou Oct. 22, 2015, 4:45 a.m. UTC
Added ovsdb_transient_datum_from_json() to avoid size check for
the diff datum that is transient in nature.
Suppose a datum contains set, and the max number of elements is 2.
If we are changing from set that contains [A, B], to a set contains
[C, D], the diff datum will contains 4 elements [A, B, C, D].

Thus diff datum should not be constrained by the size limit. However
the datum after diff is applied should not violate the size limit.

Signed-off-by: Andy Zhou <azhou@nicira.com>
---
 lib/ovsdb-data.c | 32 +++++++++++++++++++++++++++++---
 lib/ovsdb-data.h |  5 +++++
 2 files changed, 34 insertions(+), 3 deletions(-)

Comments

Ben Pfaff Nov. 10, 2015, 4:40 p.m. UTC | #1
On Wed, Oct 21, 2015 at 09:45:24PM -0700, Andy Zhou wrote:
> Added ovsdb_transient_datum_from_json() to avoid size check for
> the diff datum that is transient in nature.
> Suppose a datum contains set, and the max number of elements is 2.
> If we are changing from set that contains [A, B], to a set contains
> [C, D], the diff datum will contains 4 elements [A, B, C, D].
> 
> Thus diff datum should not be constrained by the size limit. However
> the datum after diff is applied should not violate the size limit.
> 
> Signed-off-by: Andy Zhou <azhou@nicira.com>

I'd consider doing this as a wrapper around ovsdb_datum_from_json(),
something like:

        struct ovsdb_type relaxed_type = *type;
        relaxed_type.n_min = 0;
        relaxed_type.n_max = UINT_MAX;
        return ovsdb_datum_from_json(datum, &relaxed_type, json);

This slightly reduces the duplicated code.
Andy Zhou Nov. 23, 2015, 6:50 p.m. UTC | #2
On Tue, Nov 10, 2015 at 8:40 AM, Ben Pfaff <blp@ovn.org> wrote:
> On Wed, Oct 21, 2015 at 09:45:24PM -0700, Andy Zhou wrote:
>> Added ovsdb_transient_datum_from_json() to avoid size check for
>> the diff datum that is transient in nature.
>> Suppose a datum contains set, and the max number of elements is 2.
>> If we are changing from set that contains [A, B], to a set contains
>> [C, D], the diff datum will contains 4 elements [A, B, C, D].
>>
>> Thus diff datum should not be constrained by the size limit. However
>> the datum after diff is applied should not violate the size limit.
>>
>> Signed-off-by: Andy Zhou <azhou@nicira.com>
>
> I'd consider doing this as a wrapper around ovsdb_datum_from_json(),
> something like:
>
>         struct ovsdb_type relaxed_type = *type;
>         relaxed_type.n_min = 0;
>         relaxed_type.n_max = UINT_MAX;
>         return ovsdb_datum_from_json(datum, &relaxed_type, json);
>
> This slightly reduces the duplicated code.
Good idea. I made the change as suggested. The patch is also smaller. Thanks.

Patch
diff mbox

diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c
index a62e92e..592e9ed 100644
--- a/lib/ovsdb-data.c
+++ b/lib/ovsdb-data.c
@@ -1158,7 +1158,8 @@  static struct ovsdb_error *
 ovsdb_datum_from_json__(struct ovsdb_datum *datum,
                         const struct ovsdb_type *type,
                         const struct json *json,
-                        struct ovsdb_symbol_table *symtab)
+                        struct ovsdb_symbol_table *symtab,
+                        bool check_size)
 {
     struct ovsdb_error *error;
 
@@ -1179,7 +1180,7 @@  ovsdb_datum_from_json__(struct ovsdb_datum *datum,
         }
 
         n = inner->u.array.n;
-        if (n < type->n_min || n > type->n_max) {
+        if (check_size && (n < type->n_min || n > type->n_max)) {
             return ovsdb_syntax_error(json, NULL, "%s must have %u to "
                                       "%u members but %"PRIuSIZE" are present",
                                       class, type->n_min, type->n_max, n);
@@ -1256,7 +1257,32 @@  ovsdb_datum_from_json(struct ovsdb_datum *datum,
 {
     struct ovsdb_error *error;
 
-    error = ovsdb_datum_from_json__(datum, type, json, symtab);
+    error = ovsdb_datum_from_json__(datum, type, json, symtab, true);
+    if (error) {
+        return error;
+    }
+
+    error = ovsdb_datum_sort(datum, type->key.type);
+    if (error) {
+        ovsdb_datum_destroy(datum, type);
+    }
+    return error;
+}
+
+/* Parses 'json' as a datum of the type described by 'type' for internal
+ * use. This function is similar to 'ovsdb_datum_from_json', except:
+ * the member size of set or map is not checked.
+ *
+ * The datum generated should be used then discard. It is not suitable
+ * for storing into IDL because of the possible member size violation.  */
+struct ovsdb_error *
+ovsdb_transient_datum_from_json(struct ovsdb_datum *datum,
+                                const struct ovsdb_type *type,
+                                const struct json *json)
+{
+    struct ovsdb_error *error;
+
+    error =  ovsdb_datum_from_json__(datum, type, json, NULL, false);
     if (error) {
         return error;
     }
diff --git a/lib/ovsdb-data.h b/lib/ovsdb-data.h
index e144c70..802f718 100644
--- a/lib/ovsdb-data.h
+++ b/lib/ovsdb-data.h
@@ -161,6 +161,11 @@  struct ovsdb_error *ovsdb_datum_from_json(struct ovsdb_datum *,
                                           const struct json *,
                                           struct ovsdb_symbol_table *)
     OVS_WARN_UNUSED_RESULT;
+struct ovsdb_error *ovsdb_transient_datum_from_json(
+                                          struct ovsdb_datum *,
+                                          const struct ovsdb_type *,
+                                          const struct json *)
+    OVS_WARN_UNUSED_RESULT;
 struct json *ovsdb_datum_to_json(const struct ovsdb_datum *,
                                  const struct ovsdb_type *);