Refactor the weston_output init/fini/enable/disable and weston_compositor_add/remove_output mess
The following discussion from !637 should be addressed:
The fix looks good, but ideally weston_output would be fully initialized with sane/valid values when created. The current partial initialization split between creation and enablement makes it more difficult to reason about what's valid under which scenarios. Although ensuring full valid initialization is outside the scope of this MR, perhaps we could take a first step here and move
Copying my reply:
That would be a bigger refactoring, as many fields already in
weston_output follow the enable/disable life cycle instead of init/fini life cycle. See
weston_output_enable(). These are mostly things that should semantically remain unused when the output is not enabled, i.e. their semantic life time matches enable/disable better than init/fini.
But I tend to agree. We could have more asserts to ensure that fields were properly unused at enable time and cleared at disable time. It just won't crash at the site of illegal access but at check time. Or, split
weston_output into the base struct and a dynamically allocated enabled struct.
It doesn't help that much of what
weston_output_enable() does is undone in
I think the big fat comment in
weston_output_disable() about turning non-enabled outputs off is also out-of-date nowadays. All this mess is inherited from the times when backends created outputs directly for all "connectors" they could light up, before
weston_head was introduced.
All that stuff could use a serious refactoring.