Skip to content

Commit

Permalink
Explicitly initialise all value boxes with a magic constant
Browse files Browse the repository at this point in the history
This lets all the assignment tests assume boxes are in an initialised and sane state, and NOT blindly overwrite all the flags
  • Loading branch information
arr2036 committed Sep 4, 2023
1 parent 71f5e7e commit f0760a6
Show file tree
Hide file tree
Showing 10 changed files with 68 additions and 15 deletions.
2 changes: 1 addition & 1 deletion src/lib/json/json.c
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ static int json_afrom_value_box(TALLOC_CTX *ctx, json_object **out,
{
struct json_object *obj;
fr_value_box_t const *vb;
fr_value_box_t vb_str;
fr_value_box_t vb_str = FR_VALUE_BOX_INITIALISER_NULL(vb_str);
int is_enum = 0;

fr_assert(vp);
Expand Down
2 changes: 1 addition & 1 deletion src/lib/kafka/base.c
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ static int kafka_topic_config_dflt(CONF_PAIR **out, void *parent, CONF_SECTION *

static int kafka_config_parse_single(char const **out, CONF_PAIR *cp, CONF_PARSER const *rule)
{
fr_value_box_t vb;
fr_value_box_t vb = FR_VALUE_BOX_INITIALISER_NULL(vb);
fr_kafka_conf_ctx_t const *kctx = rule->uctx;
fr_type_t type = FR_BASE_TYPE(rule->type);
static _Thread_local char buff[sizeof("18446744073709551615")];
Expand Down
6 changes: 3 additions & 3 deletions src/lib/server/tmpl_eval.c
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,8 @@ ssize_t _tmpl_to_type(void *out,
xlat_escape_legacy_t escape, void const *escape_ctx,
fr_type_t dst_type)
{
fr_value_box_t value_to_cast;
fr_value_box_t value_from_cast = { .type = FR_TYPE_NULL };
fr_value_box_t value_to_cast = FR_VALUE_BOX_INITIALISER_NULL(value_to_cast);
fr_value_box_t value_from_cast = FR_VALUE_BOX_INITIALISER_NULL(value_from_cast);
fr_value_box_t const *to_cast = &value_to_cast;
fr_value_box_t const *from_cast = &value_from_cast;

Expand Down Expand Up @@ -562,7 +562,7 @@ ssize_t _tmpl_to_atype(TALLOC_CTX *ctx, void *out,
fr_value_box_t from_cast;

fr_pair_t *vp = NULL;
fr_value_box_t value = (fr_value_box_t){};
fr_value_box_t value = FR_VALUE_BOX_INITIALISER_NULL(value);
bool needs_dup = false;

ssize_t slen = -1;
Expand Down
2 changes: 1 addition & 1 deletion src/lib/util/dict_fixup.c
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ int dict_fixup_enumv(dict_fixup_ctx_t *fctx, char const *filename, int line,
static inline CC_HINT(always_inline) int dict_fixup_enumv_apply(UNUSED dict_fixup_ctx_t *fctx, dict_fixup_enumv_t *fixup)
{
fr_dict_attr_t *da;
fr_value_box_t value;
fr_value_box_t value = FR_VALUE_BOX_INITIALISER_NULL(value);
fr_type_t type;
int ret;
fr_dict_attr_t const *da_const;
Expand Down
6 changes: 3 additions & 3 deletions src/lib/util/dict_tokenize.c
Original file line number Diff line number Diff line change
Expand Up @@ -1472,7 +1472,7 @@ static int dict_read_process_member(dict_tokenize_ctx_t *ctx, char **argv, int a
static int dict_read_process_value(dict_tokenize_ctx_t *ctx, char **argv, int argc)
{
fr_dict_attr_t *da;
fr_value_box_t value;
fr_value_box_t value = FR_VALUE_BOX_INITIALISER_NULL(value);
fr_slen_t enum_len;
fr_dict_attr_t const *parent = ctx->stack[ctx->stack_depth].da;

Expand Down Expand Up @@ -1598,7 +1598,7 @@ static int dict_read_process_struct(dict_tokenize_ctx_t *ctx, char **argv, int a
int i;
fr_dict_attr_t const *da;
fr_dict_attr_t const *parent = NULL;
fr_value_box_t value;
fr_value_box_t value = FR_VALUE_BOX_INITIALISER_NULL(value);
unsigned int attr;
fr_dict_attr_flags_t flags;
char *key_attr = argv[1];
Expand Down Expand Up @@ -2799,7 +2799,7 @@ int fr_dict_internal_afrom_file(fr_dict_t **out, char const *dict_subdir, char c
fr_dict_attr_flags_t flags = { .internal = true };
char *type_name;
fr_dict_attr_t *cast_base;
fr_value_box_t box;
fr_value_box_t box = FR_VALUE_BOX_INITIALISER_NULL(box);

if (unlikely(!dict_gctx)) {
fr_strerror_const("fr_dict_global_ctx_init() must be called before loading dictionary files");
Expand Down
20 changes: 20 additions & 0 deletions src/lib/util/dlist.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,26 @@ typedef struct {

static_assert(sizeof(unsigned int) >= 4, "Unsigned integer too small on this platform");

/** Static initialiser for entries
*
@code {.c}
fr_dlist_entry_t my_entry = FR_DLIST_INITIALISER(my_entry)
@endcode
*
* @param[in] _entry what's being initialised.
*/
#define FR_DLIST_ENTRY_INITIALISER(_entry) { .next = &_entry, .prev = &_entry }

/** Static initialiser for list heads
*
@code {.c}
fr_dlist_head_t my_head = FR_DLIST_INITIALISER(my_head)
@endcode
*
* @param[in] _entry what's being initialised.
*/
#define FR_DLIST_HEAD_INITIALISER(_head) { .entry = FR_DLIST_ENTRY_INITIALISER((_head).entry) }

/** Iterate over the contents of a list
*
* @param[in] _list_head to iterate over.
Expand Down
4 changes: 4 additions & 0 deletions src/lib/util/value.c
Original file line number Diff line number Diff line change
Expand Up @@ -5941,6 +5941,10 @@ DIAG_ON(nonnull-compare)

if (vb->talloced) vb = talloc_get_type_abort_const(vb, fr_value_box_t);

#ifndef NDEBUG
fr_fatal_assert_msg(vb->magic == FR_VALUE_BOX_MAGIC, "CONSISTENCY CHECK FAILED %s[%i]: fr_value_box_t magic "
"incorrect, expected %" PRIx64 ", got %" PRIx64, file, line, FR_VALUE_BOX_MAGIC, vb->magic);
#endif
switch (vb->type) {
case FR_TYPE_STRING:
fr_fatal_assert_msg(vb->vb_strvalue, "CONSISTENCY CHECK FAILED %s[%i]: fr_value_box_t strvalue field "
Expand Down
37 changes: 33 additions & 4 deletions src/lib/util/value.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ extern fr_sbuff_escape_rules_t *fr_value_escape_by_char[UINT8_MAX + 1];

extern fr_sbuff_escape_rules_t fr_value_escape_unprintables;

#ifndef NDEBUG
# define FR_VALUE_BOX_MAGIC RADIUSD_MAGIC_NUMBER
#endif

/** @name List and cursor type definitions
*/
FR_DLIST_TYPES(fr_value_box_list)
Expand Down Expand Up @@ -170,8 +174,9 @@ struct value_box_s {
fr_value_box_datum_t datum; //!< The value held by the value box. Should appear
///< last for packing efficiency.
#ifndef NDEBUG
char const *file; //!< File where the box was allocated (if heap allocated).
int line; //!< Line where the box was allocated (if heap allocated).
uint64_t magic; //!< Value to verify that the structure was allocated or initialised properly.
char const *file; //!< File where the box was allocated or initialised.
int line; //!< Line where the box was allocated or initialised.
#endif
};

Expand Down Expand Up @@ -209,6 +214,12 @@ typedef enum {
#define vb_should_free_value(_action) ((_action & FR_VALUE_BOX_LIST_FREE_BOX_VALUE) == FR_VALUE_BOX_LIST_FREE_BOX_VALUE)
#define vb_should_remove(_action) ((_action & FR_VALUE_BOX_LIST_REMOVE) == FR_VALUE_BOX_LIST_REMOVE)

#ifndef NDEBUG
#define VALUE_BOX_NDEBUG_INITIALISER .file = __FILE__, .line = __LINE__, .magic = FR_VALUE_BOX_MAGIC
#else
#define VALUE_BOX_NDEBUG_INITIALISER
#endif

/** @name Field accessors for #fr_value_box_t
*
* Use these instead of accessing fields directly to make refactoring
Expand Down Expand Up @@ -258,7 +269,7 @@ typedef enum {
*
* @{
*/
#define _fr_box_with_len(_type, _field, _val, _len) &(fr_value_box_t){ .type = _type, _field = _val, .vb_length = _len }
#define _fr_box_with_len(_type, _field, _val, _len) &(fr_value_box_t){ .type = _type, _field = _val, .vb_length = _len, VALUE_BOX_NDEBUG_INITIALISER }

#define fr_box_strvalue(_val) _fr_box_with_len(FR_TYPE_STRING, .vb_strvalue, _val, strlen(_val))
#define fr_box_strvalue_len(_val, _len) _fr_box_with_len(FR_TYPE_STRING, .vb_strvalue, _val, _len)
Expand All @@ -267,7 +278,7 @@ typedef enum {
#define fr_box_strvalue_buffer(_val) _fr_box_with_len(FR_TYPE_STRING, .vb_strvalue, _val, talloc_array_length(_val) - 1)
#define fr_box_octets_buffer(_val) _fr_box_with_len(FR_TYPE_OCTETS, .vb_octets, _val, talloc_array_length(_val))

#define _fr_box(_type, _field, _val) (&(fr_value_box_t){ .type = _type, _field = (_val) })
#define _fr_box(_type, _field, _val) (&(fr_value_box_t){ .type = _type, _field = (_val), VALUE_BOX_NDEBUG_INITIALISER })

#define fr_box_ipaddr(_val) _fr_box((((_val).af == AF_INET) ? \
(((_val).prefix == 32) ? FR_TYPE_IPV4_ADDR : \
Expand Down Expand Up @@ -453,6 +464,23 @@ extern fr_sbuff_parse_rules_t const *value_parse_rules_quoted_char[UINT8_MAX];
*
* @{
*/
/** A static initialiser for stack/globally allocated boxes
*
* We can only safely initialise a null box, as many other type need special initialisation
*/
#define FR_VALUE_BOX_INITIALISER_NULL(_vb) \
{ \
.type = FR_TYPE_NULL, \
.datum = { \
.children = { \
FR_DLIST_HEAD_INITIALISER((_vb).datum.children.head) \
} \
}, \
.entry = { \
.entry = FR_DLIST_ENTRY_INITIALISER((_vb).entry.entry) \
}, \
VALUE_BOX_NDEBUG_INITIALISER \
}

static inline CC_HINT(nonnull(1), always_inline)
void _fr_value_box_init(NDEBUG_LOCATION_ARGS fr_value_box_t *vb, fr_type_t type, fr_dict_attr_t const *enumv, bool tainted)
Expand Down Expand Up @@ -501,6 +529,7 @@ void _fr_value_box_init(NDEBUG_LOCATION_ARGS fr_value_box_t *vb, fr_type_t type,
}

#ifndef NDEBUG
vb->magic = FR_VALUE_BOX_MAGIC;
vb->file = file;
vb->line = line;
#endif
Expand Down
2 changes: 1 addition & 1 deletion src/protocols/dhcpv4/base.c
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ ssize_t fr_dhcpv4_encode_dbuff(fr_dbuff_t *dbuff, dhcp_packet_t *original, int c
*/
int fr_dhcpv4_global_init(void)
{
fr_value_box_t value = { .type = FR_TYPE_UINT8 };
fr_value_box_t value = *fr_box_uint8(0);
uint8_t i;

if (instance_count > 0) {
Expand Down
2 changes: 1 addition & 1 deletion src/protocols/dhcpv6/base.c
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,7 @@ void fr_dhcpv6_print_hex(FILE *fp, uint8_t const *packet, size_t packet_len)
int fr_dhcpv6_global_init(void)
{
fr_dict_attr_t const *child;
fr_value_box_t value = { .type = FR_TYPE_UINT16 };
fr_value_box_t value = *fr_box_uint16(0);

if (instance_count > 0) {
instance_count++;
Expand Down

0 comments on commit f0760a6

Please sign in to comment.