core: avoid undefined behaviour in C macro (#8)

to safely use wl_container_of with a class the class has to be no
virtual functions, no inheritance, and uniform access control (e.g all
public)

work around this by putting this into a destroywrapper struct.
This commit is contained in:
Tom Englund 2024-06-26 20:35:08 +02:00 committed by GitHub
parent 914f083741
commit c1e34fb2b7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -300,15 +300,20 @@ void parseHeader() {
const auto IFACE_CLASS_NAME_CAMEL = camelize((clientCode ? "CC_" : "C_") + iface.name); const auto IFACE_CLASS_NAME_CAMEL = camelize((clientCode ? "CC_" : "C_") + iface.name);
// begin the class // begin the class
HEADER += HEADER += std::format(R"#(
std::format(R"#( struct {}DestroyWrapper {{
wl_listener listener;
{}* parent = nullptr;
}};
class {} {{ class {} {{
public: public:
{}({}); {}({});
~{}(); ~{}();
)#", )#",
IFACE_CLASS_NAME_CAMEL, IFACE_CLASS_NAME_CAMEL, (clientCode ? "wl_resource*" : "wl_client* client, uint32_t version, uint32_t id"), IFACE_CLASS_NAME_CAMEL); IFACE_CLASS_NAME_CAMEL, IFACE_CLASS_NAME_CAMEL, IFACE_CLASS_NAME_CAMEL, IFACE_CLASS_NAME_CAMEL,
(clientCode ? "wl_resource*" : "wl_client* client, uint32_t version, uint32_t id"), IFACE_CLASS_NAME_CAMEL);
if (!clientCode) { if (!clientCode) {
HEADER += std::format(R"#( HEADER += std::format(R"#(
@ -471,10 +476,10 @@ class {} {{
wl_resource* pResource = nullptr; wl_resource* pResource = nullptr;
wl_listener resourceDestroyListener; {}DestroyWrapper resourceDestroyListener;
void* pData = nullptr;)#", void* pData = nullptr;)#",
IFACE_CLASS_NAME_CAMEL); IFACE_CLASS_NAME_CAMEL, IFACE_CLASS_NAME_CAMEL);
} else { } else {
HEADER += R"#( HEADER += R"#(
wl_resource* pResource = nullptr; wl_resource* pResource = nullptr;
@ -614,11 +619,12 @@ static void {}(void* data, void* resource{}) {{
if (!clientCode) { if (!clientCode) {
SOURCE += std::format(R"#( SOURCE += std::format(R"#(
static void _{}__DestroyListener(wl_listener* l, void* d) {{ static void _{}__DestroyListener(wl_listener* l, void* d) {{
{}* pResource = wl_container_of(l, pResource, resourceDestroyListener); {}DestroyWrapper *wrap = wl_container_of(l, wrap, listener);
{}* pResource = wrap->parent;
pResource->onDestroyCalled(); pResource->onDestroyCalled();
}} }}
)#", )#",
IFACE_CLASS_NAME_CAMEL, IFACE_CLASS_NAME_CAMEL); IFACE_CLASS_NAME_CAMEL, IFACE_CLASS_NAME_CAMEL, IFACE_CLASS_NAME_CAMEL);
} }
// create vtable // create vtable
@ -689,9 +695,9 @@ void {}::{}({}) {{
auto proxy = wl_proxy_marshal_flags((wl_proxy*)pResource, {}, {}, wl_proxy_get_version((wl_proxy*)pResource), {}{}{});{} auto proxy = wl_proxy_marshal_flags((wl_proxy*)pResource, {}, {}, wl_proxy_get_version((wl_proxy*)pResource), {}{}{});{}
}} }}
)#", )#",
ptrRetType, IFACE_CLASS_NAME_CAMEL, EVENT_NAME, argsC, (ev.newIdType.empty() ? "" : " nullptr"), (ev.destructor ? "\n destroyed = true;" : ""), evid, ptrRetType, IFACE_CLASS_NAME_CAMEL, EVENT_NAME, argsC, (ev.newIdType.empty() ? "" : " nullptr"),
(ev.newIdType.empty() ? "nullptr" : "&" + ev.newIdType + "_interface"), flags, (!ev.newIdType.empty() ? ", nullptr" : ""), argsN, (ev.destructor ? "\n destroyed = true;" : ""), evid, (ev.newIdType.empty() ? "nullptr" : "&" + ev.newIdType + "_interface"), flags,
(ev.newIdType.empty() ? "\n proxy;" : "\n\n return proxy;")); (!ev.newIdType.empty() ? ", nullptr" : ""), argsN, (ev.newIdType.empty() ? "\n proxy;" : "\n\n return proxy;"));
} }
evid++; evid++;
@ -835,16 +841,17 @@ const wl_interface {} = {{
return; return;
wl_resource_set_user_data(pResource, this); wl_resource_set_user_data(pResource, this);
wl_list_init(&resourceDestroyListener.link); wl_list_init(&resourceDestroyListener.listener.link);
resourceDestroyListener.notify = _{}__DestroyListener; resourceDestroyListener.listener.notify = _{}__DestroyListener;
wl_resource_add_destroy_listener(pResource, &resourceDestroyListener); resourceDestroyListener.parent = this;
wl_resource_add_destroy_listener(pResource, &resourceDestroyListener.listener);
wl_resource_set_implementation(pResource, {}, this, nullptr); wl_resource_set_implementation(pResource, {}, this, nullptr);
}} }}
{}::~{}() {{ {}::~{}() {{
wl_list_remove(&resourceDestroyListener.link); wl_list_remove(&resourceDestroyListener.listener.link);
wl_list_init(&resourceDestroyListener.link); wl_list_init(&resourceDestroyListener.listener.link);
// if we still own the wayland resource, // if we still own the wayland resource,
// it means we need to destroy it. // it means we need to destroy it.
@ -856,8 +863,8 @@ const wl_interface {} = {{
void {}::onDestroyCalled() {{ void {}::onDestroyCalled() {{
wl_resource_set_user_data(pResource, nullptr); wl_resource_set_user_data(pResource, nullptr);
wl_list_remove(&resourceDestroyListener.link); wl_list_remove(&resourceDestroyListener.listener.link);
wl_list_init(&resourceDestroyListener.link); wl_list_init(&resourceDestroyListener.listener.link);
// set the resource to nullptr, // set the resource to nullptr,
// as it will be freed. If the consumer does not destroy this resource // as it will be freed. If the consumer does not destroy this resource