BIRD crashing when --enable-debug is set

[Ondrej Zajicek]

On Sun, Nov 22, 2020 at 08:49:30PM +0100, Maria Matějka wrote:
> Hello!
> 
> Just a quick reply from my phone. When adding these checks, I also wanted to zero out slab objects but then I realized that these objects should be initialized anyway after allocation and in most cases all of these would be rewritten twice.

Hello

I noticed this issue in one of earlier bugreports and IMHO most foolproof
way is just to use sl_allocz() everywhere. It is true that allocated
structures would be rewritten twice, but these allocated from slab are
small enough that likely it does not matter.

The advantage is that we can easily check it by one grep for whole
codebase (instead of reviewing and manually updating all existing code)
and that it is less likely that people not acquainted with our style use
it in a bad way.


> I'm not a huge fan of the struct assignment to pointers thing (there's
> the thing where it doesn't zero out padding, but mostly I just think
> it's ugly :)). However, I do see it is widely used in the Bird codebase,

I like and use struct assignment to initialize local/stack variables
(instead of manual memset()).

For allocated structures, i have no strong preference for either
'classical' per-field initialization, or initialization using struct
assignment, as long as *allocz() is used for allocation. Sometimes the
first variant is more useful (as it naturally allows more structured
statements), sometimes the second.

There are plenty of code in Bird where the first variant is used and i do
not think it make sense to convert all of it to the second variant just
to ensure proper initialization, when *alloc() -> *allocz() would do
the same thing.


> The preferred way of using slab objects is therefore a full init by structure assignment after alloc like this:
> 
> struct foo *f = sl_alloc(...);
> *f = (struct foo) {...};
> 
> In cases of other allocations, there are allocz variants to zero the allocated memory instead of having to call memset, yet slabs are intended to be a fixed-size structure allocator which corresponds to a possibility of direct structure assignment.
> 
> I hope this is sufficient explanation. Feel free to dispute it or discuss anyway, I may be wrong somehow.
> 
> Maria

-- 
Elen sila lumenn' omentielvo

Ondrej 'Santiago' Zajicek (email: santiago at crfreenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."