rx: Make rx_opaque_free idempotent 44/13944/2
authorAndrew Deason <adeason@sinenomine.net>
Wed, 21 Aug 2019 17:43:03 +0000 (12:43 -0500)
committerBenjamin Kaduk <kaduk@mit.edu>
Fri, 20 Dec 2019 16:59:18 +0000 (11:59 -0500)
Currently rx_opaque_free sets the given argument to NULL, a style that
helps prevent double-frees. However, it doesn't check if the given
buffer is already NULL, which makes potential callers that use a 'goto
done'-style cleanup block do something like:

 done:
    if (buf)
        rx_opaque_free(&buf);

To avoid the extra if(), make rx_opaque_free a no-op if it's given a
NULL buffer, similar to how free(NULL) is a no-op on most platforms.

Slightly refactor how we reference our argument as well, to limit the
number of layers of indirection the code needs to deal with.

Do the same for rx_opaque_zeroFree.

Note that there are currently no callers of
rx_opaque_free/rx_opaque_zeroFree, but future commits will add some.

Change-Id: Ic86a9c63903bebbddd311912cfbcb61198e3f0b0
Reviewed-on: https://gerrit.openafs.org/13944
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

src/rx/rx_opaque.c

index 85c7d47..a6353b1 100644 (file)
@@ -174,11 +174,16 @@ rx_opaque_zeroFreeContents(struct rx_opaque *buf)
  */
 
 void
-rx_opaque_free(struct rx_opaque **buf)
+rx_opaque_free(struct rx_opaque **a_buf)
 {
-    rx_opaque_freeContents(*buf);
-    rxi_Free(*buf, sizeof(**buf));
-    *buf = NULL;
+    struct rx_opaque *buf = *a_buf;
+    *a_buf = NULL;
+    if (buf == NULL) {
+       return;
+    }
+
+    rx_opaque_freeContents(buf);
+    rxi_Free(buf, sizeof(*buf));
 }
 
 /*!
@@ -189,9 +194,14 @@ rx_opaque_free(struct rx_opaque **buf)
  */
 
 void
-rx_opaque_zeroFree(struct rx_opaque **buf)
+rx_opaque_zeroFree(struct rx_opaque **a_buf)
 {
-    rx_opaque_zeroFreeContents(*buf);
-    rxi_Free(*buf, sizeof(**buf));
-    *buf = NULL;
+    struct rx_opaque *buf = *a_buf;
+    *a_buf = NULL;
+    if (buf == NULL) {
+       return;
+    }
+
+    rx_opaque_zeroFreeContents(buf);
+    rxi_Free(buf, sizeof(*buf));
 }