diff options
author | David Lamparter <equinox@opensourcerouting.org> | 2022-07-06 10:48:56 +0200 |
---|---|---|
committer | David Lamparter <equinox@opensourcerouting.org> | 2022-07-06 10:48:56 +0200 |
commit | 255a76a56a154b1f26e3971aeb3f6111ce9a84a7 (patch) | |
tree | c83614f22712861c771f28cedd0938411ea98379 /lib | |
parent | Merge pull request #11534 from mjstapp/fix_typesafe_sa (diff) | |
download | frr-255a76a56a154b1f26e3971aeb3f6111ce9a84a7.tar.xz frr-255a76a56a154b1f26e3971aeb3f6111ce9a84a7.zip |
lib: use assume() for SA fixing, add explainer
Literally 4 minutes after hitting merge on Mark's previous fix for this
I remembered we have an `assume()` macro in compiler.h which seems like
the right tool for this.
(... and if I'm touching it, I might as well add a little text
explaining what's going on here.)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Diffstat (limited to 'lib')
-rw-r--r-- | lib/typesafe.h | 22 |
1 files changed, 20 insertions, 2 deletions
diff --git a/lib/typesafe.h b/lib/typesafe.h index df963f530..50c410ad2 100644 --- a/lib/typesafe.h +++ b/lib/typesafe.h @@ -308,9 +308,27 @@ struct dlist_head { static inline void typesafe_dlist_add(struct dlist_head *head, struct dlist_item *prev, struct dlist_item *item) { + /* SA on clang-11 thinks this can happen, but in reality -assuming no + * memory corruption- it can't. DLIST uses a "closed" ring, i.e. the + * termination at the end of the list is not NULL but rather a pointer + * back to the head. (This eliminates special-casing the first or last + * item.) + * + * Sadly, can't use assert() here since the libfrr assert / xref code + * uses typesafe lists itself... that said, if an assert tripped here + * we'd already be way past some memory corruption, so we might as + * well just take the SEGV. (In the presence of corruption, we'd see + * random SEGVs from places that make no sense at all anyway, an + * assert might actually be a red herring.) + * + * ("assume()" tells the compiler to produce code as if the condition + * will always hold; it doesn't have any actual effect here, it'll + * just SEGV out on "item->next->prev = item".) + */ + assume(prev->next != NULL); + item->next = prev->next; - if (item->next) - item->next->prev = item; + item->next->prev = item; item->prev = prev; prev->next = item; head->count++; |