mate-desktop / mate-media

Media tools for MATE
https://mate-desktop.org
GNU General Public License v2.0
19 stars 25 forks source link

Fix various instance checks #208

Closed cwendling closed 1 year ago

cwendling commented 1 year ago

The g_return*_if_fail() calls should use PREFIX_IS_TYPE_NAME [1], which returns whether the given instance is of the right type, not PREFIX_TYPE_NAME [2], which checks whether the given instance if of the right type, warns if not, and returns the passed-in instance unchanged.

Using the wrong one means the returned value is the passed-in pointer, and thus passes the g_return*_if_fail() whenever the pointer is non-NULL, no matter whether it's of the right type or not.

A warning would still be emitted by g_type_check_instance_cast(), but the execution wouldn't get short-circuited.

[1] calls g_type_check_instance_is_a() [2] calls g_type_check_instance_cast()

raveit65 commented 1 year ago

Is this a bugfix, btw. can this reduce current crashes with pulseaudio?

cwendling commented 1 year ago

Is this a bugfix, btw. can this reduce current crashes with pulseaudio?

That's the initial reason for me doing it, but I have no idea if this will actually help those issues in particular or not. AFAICT, before this change we still should have had warnings in case it should have short-circuited the call, so logs should have had something in them right before the crash. If they didn't, it's unlikely this will make any difference.

raveit65 commented 1 year ago

Ok, than it make sense to push to stable 1.26 if you agree.

cwendling commented 1 year ago

@raveit65 yes, sure, and I think it's pretty safe indeed (and that's an understatement).