From 8a27e591b9bf45a9c7b6f0cb3bfa727382c4958c Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 3 Mar 2023 16:49:33 -0800 Subject: [PATCH] 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. --- src/data/caseproto.c | 51 +++++++++++++++++++++---------------------- src/data/caseproto.h | 17 +++++---------- src/data/dictionary.c | 19 ++++++++++------ 3 files changed, 42 insertions(+), 45 deletions(-) diff --git a/src/data/caseproto.c b/src/data/caseproto.c index f328ca130c..358d59eb3d 100644 --- 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 244ea418e0..8af1b0b491 100644 --- 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 d69f1c2901..7a4d094e78 100644 --- 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; } -- 2.30.2