Skip to content

Commit

Permalink
Revise a couple of uses of FR_TYPE_STRUCTURAL
Browse files Browse the repository at this point in the history
FR_TYPE_STRUCTURAL has a name of the same form as the values of
fr_type_t, but is carefully #defined so it can appear in a switch
statement looking like a single value but underneath expanding to
multiple cases, making some of its uses counterintuitive.
  • Loading branch information
jejones3141 committed Nov 16, 2023
1 parent c835967 commit 2f51124
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 31 deletions.
33 changes: 5 additions & 28 deletions src/lib/util/dict_util.c
Original file line number Diff line number Diff line change
Expand Up @@ -1114,40 +1114,17 @@ int dict_attr_child_add(fr_dict_attr_t *parent, fr_dict_attr_t *child)
* Attributes are inserted into the bin in order of their attribute
* numbers to allow slightly more efficient lookups.
*/
bin = &children[child->attr & 0xff];
for (;;) {
bool child_is_struct = false;
bool bin_is_struct = false;

if (!*bin) break;

for (bin = &children[child->attr & 0xff]; *bin; bin = &(*bin)->next) {
/*
* Workaround for vendors that overload the RFC space.
* Structural attributes always take priority.
*/
switch (child->type) {
case FR_TYPE_STRUCTURAL:
child_is_struct = true;
break;

default:
break;
}

switch ((*bin)->type) {
case FR_TYPE_STRUCTURAL:
bin_is_struct = true;
break;

default:
break;
}
bool child_is_struct = fr_type_is_structural(child->type);
bool bin_is_struct = fr_type_is_structural((*bin)->type);

if (child_is_struct && !bin_is_struct) break;
else if (fr_dict_vendor_num_by_da(child) <= fr_dict_vendor_num_by_da(*bin)) break; /* Prioritise RFC attributes */
else if (child->attr <= (*bin)->attr) break;

bin = &(*bin)->next;
if (fr_dict_vendor_num_by_da(child) <= fr_dict_vendor_num_by_da(*bin)) break; /* Prioritise RFC attributes */
if (child->attr <= (*bin)->attr) break;
}

memcpy(&this, &bin, sizeof(this));
Expand Down
14 changes: 14 additions & 0 deletions src/lib/util/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,19 @@ typedef enum {
_mid(FR_TYPE_TLV) \
_end(FR_TYPE_VENDOR)

/** Hack for truthiness check
*
* - VSAs
* - Structs
* - TLVs
* - Vendors
*/
#define FR_TYPE_STRUCTURAL_EXCEPT_GROUP_DEF(_beg, _mid, _end) \
_beg(FR_TYPE_VSA) \
_mid(FR_TYPE_STRUCT) \
_mid(FR_TYPE_TLV) \
_end(FR_TYPE_VENDOR)

/** Match all non value types in case statements
*
* - Groups
Expand Down Expand Up @@ -279,6 +292,7 @@ typedef enum {
#define FR_TYPE_QUOTED FR_TYPE_QUOTED_DEF(CASE_BEG, CASE_MID, CASE_END)

#define FR_TYPE_STRUCTURAL_EXCEPT_VSA FR_TYPE_STRUCTURAL_EXCEPT_VSA_DEF(CASE_BEG, CASE_MID, CASE_END)
#define FR_TYPE_STRUCTURAL_EXCEPT_GROUP FR_TYPE_STRUCTURAL_EXCEPT_GROUP_DEF(CASE_BEG, CASE_MID, CASE_END)
#define FR_TYPE_STRUCTURAL FR_TYPE_STRUCTURAL_DEF(CASE_BEG, CASE_MID, CASE_END)
#define FR_TYPE_LEAF FR_TYPE_LEAF_DEF(CASE_BEG, CASE_MID, CASE_END)
#define FR_TYPE_NON_LEAF FR_TYPE_NON_LEAF_DEF(CASE_BEG, CASE_MID, CASE_END)
Expand Down
6 changes: 3 additions & 3 deletions src/lib/util/value.c
Original file line number Diff line number Diff line change
Expand Up @@ -6063,11 +6063,11 @@ bool fr_value_box_is_truthy(fr_value_box_t const *in)

switch (in->type) {
case FR_TYPE_NULL:
case FR_TYPE_STRUCTURAL_EXCEPT_GROUP:
return false;

case FR_TYPE_STRUCTURAL:
if (in->type == FR_TYPE_GROUP) return (fr_value_box_list_num_elements(&in->vb_group) > 0);
return false;
case FR_TYPE_GROUP:
return (fr_value_box_list_num_elements(&in->vb_group) > 0);

case FR_TYPE_BOOL:
return in->vb_bool;
Expand Down

0 comments on commit 2f51124

Please sign in to comment.