Always return NULL from the _unref() calls

There's no good reason to return the object from the unref() call other than
for testing purposes, and we can work around this by using valgrind instead.
The API is a lot more consistent when we always return NULL.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

src/libratbag.c

@@ -436,10 +436,8 @@ ratbag_device_unref(struct ratbag_device *device)
 
 	assert(device->refcount > 0);
 	device->refcount--;
-	if (device->refcount > 0)
-		return device;
-
-	ratbag_device_destroy(device);
+	if (device->refcount == 0)
+		ratbag_device_destroy(device);
 
 	return NULL;
 }
@@ -521,11 +519,10 @@ ratbag_unref(struct ratbag *ratbag)
 
 	assert(ratbag->refcount > 0);
 	ratbag->refcount--;
-	if (ratbag->refcount > 0)
-		return ratbag;
-
-	ratbag->udev = udev_unref(ratbag->udev);
-	free(ratbag);
+	if (ratbag->refcount == 0) {
+		ratbag->udev = udev_unref(ratbag->udev);
+		free(ratbag);
+	}
 
 	return NULL;
 }
@@ -636,19 +633,15 @@ ratbag_profile_destroy(struct ratbag_profile *profile)
 LIBRATBAG_EXPORT struct ratbag_profile *
 ratbag_profile_unref(struct ratbag_profile *profile)
 {
-	struct ratbag_profile *p = NULL;
-
 	if (profile == NULL)
 		return NULL;
 
 	assert(profile->refcount > 0);
 	profile->refcount--;
-	if (profile->refcount > 0)
-		p = profile;
 
 	ratbag_device_unref(profile->device);
 
-	return p;
+	return NULL;
 }
 
 LIBRATBAG_EXPORT struct ratbag_profile *
@@ -806,19 +799,15 @@ ratbag_resolution_ref(struct ratbag_resolution *resolution)
 LIBRATBAG_EXPORT struct ratbag_resolution *
 ratbag_resolution_unref(struct ratbag_resolution *resolution)
 {
-	struct ratbag_resolution *r = NULL;
-
 	if (resolution == NULL)
 		return NULL;
 
 	assert(resolution->refcount > 0);
 	resolution->refcount--;
-	if (resolution->refcount > 0)
-		r = resolution;
 
 	ratbag_profile_unref(resolution->profile);
 
-	return r;
+	return NULL;
 }
 
 LIBRATBAG_EXPORT int
@@ -1108,19 +1097,15 @@ ratbag_button_destroy(struct ratbag_button *button)
 LIBRATBAG_EXPORT struct ratbag_button *
 ratbag_button_unref(struct ratbag_button *button)
 {
-	struct ratbag_button *b = NULL;
-
 	if (button == NULL)
 		return NULL;
 
 	assert(button->refcount > 0);
 	button->refcount--;
-	if (button->refcount > 0)
-		b = button;
 
 	ratbag_profile_unref(button->profile);
 
-	return b;
+	return NULL;
 }
 
 LIBRATBAG_EXPORT void

src/libratbag.h

@@ -347,12 +347,12 @@ ratbag_device_ref(struct ratbag_device *device);
 /**
  * @ingroup device
  *
- * Dereference the ratbag device. After this, the device may have been
- * destroyed, if the last reference was dereferenced. If so, the device is
- * invalid and may not be interacted with.
+ * Dereference the ratbag device. When the internal refcount reaches
+ * zero, all resources associated with this object are released. The object
+ * must be considered invalid once unref is called.
  *
  * @param device A previously initialized ratbag device
- * @return NULL if device was destroyed otherwise the passed device
+ * @return Always NULL
  */
 struct ratbag_device *
 ratbag_device_unref(struct ratbag_device *device);
@@ -505,12 +505,12 @@ ratbag_profile_ref(struct ratbag_profile *profile);
 /**
  * @ingroup profile
  *
- * Dereference the ratbag profile. After this, the profile may have been
- * destroyed, if the last reference was dereferenced. If so, the profile is
- * invalid and may not be interacted with.
+ * Dereference the ratbag profile. When the internal refcount reaches
+ * zero, all resources associated with this object are released. The object
+ * must be considered invalid once unref is called.
  *
  * @param profile A previously initialized ratbag profile
- * @return NULL if context was destroyed otherwise the passed profile
+ * @return Always NULL
  */
 struct ratbag_profile *
 ratbag_profile_unref(struct ratbag_profile *profile);
@@ -671,12 +671,12 @@ ratbag_resolution_ref(struct ratbag_resolution *resolution);
 /**
  * @ingroup resolution
  *
- * Dereference the ratbag resolution. After this, the resolution may have been
- * destroyed, if the last reference was dereferenced. If so, the resolution is
- * invalid and may not be interacted with.
+ * Dereference the ratbag resolution. When the internal refcount reaches
+ * zero, all resources associated with this object are released. The object
+ * must be considered invalid once unref is called.
  *
  * @param resolution A previously initialized ratbag resolution
- * @return NULL if the resolution was destroyed otherwise the passed resolution
+ * @return Always NULL
  */
 struct ratbag_resolution *
 ratbag_resolution_unref(struct ratbag_resolution *resolution);
@@ -1392,12 +1392,12 @@ ratbag_button_ref(struct ratbag_button *button);
 /**
  * @ingroup button
  *
- * Dereference the ratbag button. After this, the button may have been
- * destroyed, if the last reference was dereferenced. If so, the button is
- * invalid and may not be interacted with.
+ * Dereference the ratbag button. When the internal refcount reaches
+ * zero, all resources associated with this object are released. The object
+ * must be considered invalid once unref is called.
  *
  * @param button A previously initialized ratbag button
- * @return NULL if the button was destroyed otherwise the passed button
+ * @return Always NULL
  */
 struct ratbag_button *
 ratbag_button_unref(struct ratbag_button *button);

test/test-context.c

@@ -107,9 +107,9 @@ START_TEST(context_ref)
 	ck_assert_ptr_eq(lr, lr2);
 
 	lr2 = ratbag_unref(lr2);
-	ck_assert_ptr_eq(lr2, lr);
+	ck_assert_ptr_eq(lr2, NULL);
 
-	lr2 = ratbag_unref(lr2);
+	lr2 = ratbag_unref(lr);
 	ck_assert_ptr_eq(lr2, NULL);
 }
 END_TEST

test/test-device.c

@@ -144,13 +144,13 @@ END_TEST
 	 } \
 	 for (i = 0; i <= 256; i++) { \
 		 tmp = T##_unref(a); \
-		 ck_assert(tmp == a); \
+		 ck_assert(tmp == NULL); \
 	 } \
 	 for (i = 0; i <= 256; i++) { \
 		 tmp = T##_ref(a); \
 		 ck_assert(tmp == a); \
 		 tmp = T##_unref(a); \
-		 ck_assert(tmp == a); \
+		 ck_assert(tmp == NULL); \
 	 } \
  }
 
@@ -183,7 +183,7 @@ START_TEST(device_free_context_before_device)
 	ck_assert(d != NULL);
 
 	r = ratbag_unref(r);
-	ck_assert(r != NULL);
+	ck_assert(r == NULL);
 
 	d = ratbag_device_unref(d);
 	ck_assert(d == NULL);
@@ -465,8 +465,8 @@ START_TEST(device_freed_before_profile)
 	ck_assert_int_eq(is_active, 0);
 
 	d = ratbag_device_unref(d);
-	/* a ref to d is still kept through p, so d can not be NULL */
-	ck_assert(d != NULL);
+	/* a ref to d is still kept through p */
+	ck_assert(d == NULL);
 
 	rc = ratbag_profile_set_active(p);
 	ck_assert_int_eq(rc, 0);
@@ -500,15 +500,15 @@ START_TEST(device_and_profile_freed_before_button)
 	ck_assert(p != NULL);
 
 	d = ratbag_device_unref(d);
-	/* a ref to d is still kept through p, so d can not be NULL */
-	ck_assert(d != NULL);
+	/* a ref to d is still kept through p */
+	ck_assert(d == NULL);
 
 	b = ratbag_profile_get_button(p, 0);
 	ck_assert(b != NULL);
 
 	p = ratbag_profile_unref(p);
-	/* a ref to p is still kept through b, so p can not be NULL */
-	ck_assert(p != NULL);
+	/* a ref to p is still kept through b */
+	ck_assert(p == NULL);
 
 	/* FIXME: should probably call something for the button */
 	b = ratbag_button_unref(b);
@@ -539,15 +539,15 @@ START_TEST(device_and_profile_freed_before_resolution)
 	ck_assert(p != NULL);
 
 	d = ratbag_device_unref(d);
-	/* a ref to d is still kept through p, so d can not be NULL */
-	ck_assert(d != NULL);
+	/* a ref to d is still kept through p */
+	ck_assert(d == NULL);
 
 	res = ratbag_profile_get_resolution(p, 0);
 	ck_assert(res != NULL);
 
 	p = ratbag_profile_unref(p);
-	/* a ref to p is still kept through res, so p can not be NULL */
-	ck_assert(p != NULL);
+	/* a ref to p is still kept through res */
+	ck_assert(p == NULL);
 
 	/* FIXME: should probably call something for the resolution */
 	res = ratbag_resolution_unref(res);
@@ -580,7 +580,7 @@ START_TEST(device_and_profile_and_button_freed_before_resolution)
 
 	d = ratbag_device_unref(d);
 	/* a ref to d is still kept through p, so d can not be NULL */
-	ck_assert(d != NULL);
+	ck_assert(d == NULL);
 
 	res = ratbag_profile_get_resolution(p, 0);
 	ck_assert(res != NULL);
@@ -589,8 +589,8 @@ START_TEST(device_and_profile_and_button_freed_before_resolution)
 	ck_assert(b != NULL);
 
 	p = ratbag_profile_unref(p);
-	/* a ref to p is still kept through res and b, so p can not be NULL */
-	ck_assert(p != NULL);
+	/* a ref to p is still kept through res and b */
+	ck_assert(p == NULL);
 
 	/* a ref to p is still in res */
 	b = ratbag_button_unref(b);
@@ -626,8 +626,8 @@ START_TEST(device_and_profile_and_resolution_freed_before_button)
 	ck_assert(p != NULL);
 
 	d = ratbag_device_unref(d);
-	/* a ref to d is still kept through p, so d can not be NULL */
-	ck_assert(d != NULL);
+	/* a ref to d is still kept through p */
+	ck_assert(d == NULL);
 
 	res = ratbag_profile_get_resolution(p, 0);
 	ck_assert(res != NULL);
@@ -636,8 +636,8 @@ START_TEST(device_and_profile_and_resolution_freed_before_button)
 	ck_assert(b != NULL);
 
 	p = ratbag_profile_unref(p);
-	/* a ref to p is still kept through res and b, so p can not be NULL */
-	ck_assert(p != NULL);
+	/* a ref to p is still kept through res and b */
+	ck_assert(p == NULL);
 
 	/* a ref to p is still in res */
 	res = ratbag_resolution_unref(res);