caseproto: Disallow -1 elements in caseprotos.

This is a stronger invariant that should make PSPP code easier to
understand.

It required updating dict_get_proto() to avoid intermediate stages in
which a -1 appeared as a width.

diff --git a/src/data/caseproto.c b/src/data/caseproto.c
index f328ca130c1eb88385af9abb5fb8428a0d084e8c..358d59eb3d37cb01dd2a13dc358461c8172d44d6 100644 (file)
--- a/src/data/caseproto.c
+++ b/src/data/caseproto.c
@@ -48,6 +48,20 @@ caseproto_create (void)
   return proto;
 }
 
+struct caseproto * MALLOC_LIKE
+caseproto_from_widths (short int *widths, size_t n)
+{
+  struct caseproto *proto = xmalloc (sizeof *proto);
+  *proto = (struct caseproto) {
+    .ref_cnt = 1,
+    .n_widths = n,
+    .allocated_widths = n,
+    .widths = widths,
+  };
+  proto->n_strings = count_strings (proto, 0, n);
+  return proto;
+}
+
 static void
 do_unref (void *proto_)
 {
@@ -65,27 +79,11 @@ caseproto_ref_pool (const struct caseproto *proto_, struct pool *pool)
   return proto;
 }
 
-/* Returns a replacement for PROTO that is unshared and has
-   enough room for at least N_WIDTHS widths before additional
-   memory is needed.  */
-struct caseproto *
-caseproto_reserve (struct caseproto *proto, size_t n_widths)
-{
-  proto = caseproto_unshare (proto);
-  if (n_widths > proto->allocated_widths)
-    {
-      proto->allocated_widths = MAX (proto->allocated_widths * 2, n_widths);
-      proto->widths = xnrealloc (proto->widths, proto->allocated_widths,
-                                 sizeof *proto->widths);
-    }
-  return proto;
-}
-
 /* Returns a replacement for PROTO with WIDTH appended.  */
 struct caseproto *
 caseproto_add_width (struct caseproto *proto, int width)
 {
-  assert (width >= -1 && width <= MAX_STRING);
+  assert (width >= 0 && width <= MAX_STRING);
 
   proto = caseproto_unshare (proto);
   if (proto->n_widths >= proto->allocated_widths)
@@ -98,18 +96,15 @@ caseproto_add_width (struct caseproto *proto, int width)
   return proto;
 }
 
-/* Returns a replacement for PROTO with the width at index IDX
-   replaced by WIDTH.  IDX may be greater than the current number
-   of widths in PROTO, in which case any gap is filled in by
-   widths of -1. */
+/* Returns a replacement for PROTO with the width at index IDX replaced by
+   WIDTH.  */
 struct caseproto *
 caseproto_set_width (struct caseproto *proto, size_t idx, int width)
 {
-  assert (width >= -1 && width <= MAX_STRING);
+  assert (idx < proto->n_widths);
+  assert (width >= 0 && width <= MAX_STRING);
 
-  proto = caseproto_reserve (proto, idx + 1);
-  while (idx >= proto->n_widths)
-    proto->widths[proto->n_widths++] = -1;
+  proto = caseproto_unshare (proto);
   proto->n_strings -= count_strings (proto, idx, 1);
   proto->widths[idx] = width;
   proto->n_strings += count_strings (proto, idx, 1);
@@ -123,9 +118,13 @@ caseproto_set_width (struct caseproto *proto, size_t idx, int width)
 struct caseproto *
 caseproto_insert_width (struct caseproto *proto, size_t before, int width)
 {
+  assert (width >= 0 && width <= MAX_STRING);
   assert (before <= proto->n_widths);
 
-  proto = caseproto_reserve (proto, proto->n_widths + 1);
+  proto = caseproto_unshare (proto);
+  if (proto->n_widths >= proto->allocated_widths)
+    proto->widths = x2nrealloc (proto->widths, &proto->allocated_widths,
+                                sizeof *proto->widths);
   proto->n_strings += value_needs_init (width);
   insert_element (proto->widths, proto->n_widths, sizeof *proto->widths,
                   before);

diff --git a/src/data/caseproto.h b/src/data/caseproto.h
index 244ea418e0c02e02671e3dc2714624b9f63b2a69..8af1b0b4919e18af6df9ed834990ed2422ddef4d 100644 (file)
--- a/src/data/caseproto.h
+++ b/src/data/caseproto.h
@@ -31,15 +31,9 @@
    A case prototype specifies the number and type of the values
    in a case.  It is essentially an array of integers, where the
    array index is an index into a case and each element
-   represents the width of a value in a case.  Valid widths are:
-
-       * 0, indicating a numeric value.
-
-       * A positive integer between 1 and 32767, indicating the
-         size in bytes of a string value.
-
-       * -1, indicating that the value at this index in the case
-         is not used at all.  (This is rarely useful.)
+   represents the width of a value in a case.  A width of 0
+   indicates a numeric value, and any positive integer up to
+   MAX_STRING indicate the size in bytes of a string value.
 
    Case prototypes are reference counted.  A newly created case
    prototype has a single owner (the code that created it),
@@ -51,9 +45,7 @@
    piece of code that incremented the reference count.
 
    Functions that modifying case prototypes automatically unshare
-   them as necessary.  All of these functions potentially move
-   the caseproto around in memory even when the case prototype is
-   not shared.  Thus it is very important that every caller of a
+   them as necessary.  Thus it is very important that every caller of a
    function that modifies a case prototype thereafter uses the returned
    caseproto instead of the one passed in as an argument.
 
@@ -79,6 +71,7 @@ struct pool;
 
 /* Creation and destruction. */
 struct caseproto *caseproto_create (void) MALLOC_LIKE;
+struct caseproto *caseproto_from_widths (short int *, size_t n) MALLOC_LIKE;
 static inline struct caseproto *caseproto_ref (const struct caseproto *) WARN_UNUSED_RESULT;
 struct caseproto *caseproto_ref_pool (const struct caseproto *, struct pool *) WARN_UNUSED_RESULT;
 static inline void caseproto_unref (struct caseproto *);

diff --git a/src/data/dictionary.c b/src/data/dictionary.c
index d69f1c29017c61c7d4842de8e11cce765f757c9e..7a4d094e78994d45f81cfa14836b4dd2e4e85f7b 100644 (file)
--- a/src/data/dictionary.c
+++ b/src/data/dictionary.c
@@ -1401,14 +1401,19 @@ dict_get_proto (const struct dictionary *d_)
   struct dictionary *d = CONST_CAST (struct dictionary *, d_);
   if (d->proto == NULL)
     {
-      size_t i;
+      short int *widths = xnmalloc (d->n_vars, sizeof *widths);
+      for (size_t i = 0; i < d->n_vars; i++)
+        widths[i] = -1;
+      for (size_t i = 0; i < d->n_vars; i++)
+        {
+          const struct variable *var = d->vars[i].var;
+          size_t case_idx = var_get_case_index (var);
+          assert (case_idx < d->n_vars);
+          assert (widths[case_idx] == -1);
+          widths[case_idx] = var_get_width (var);
+        }
 
-      d->proto = caseproto_create ();
-      d->proto = caseproto_reserve (d->proto, d->n_vars);
-      for (i = 0; i < d->n_vars; i++)
-        d->proto = caseproto_set_width (d->proto,
-                                        var_get_case_index (d->vars[i].var),
-                                        var_get_width (d->vars[i].var));
+      d->proto = caseproto_from_widths (widths, d->n_vars);
     }
   return d->proto;
 }