[PATCH 2.3.1] Wombat: Add wombatQueue_deallocate


Sam Wilson <Sam.Wilson@...>
 

Hey,

Here is a patch that adds wombatQueue_deallocate, which makes it
possible to reclaim resources if wombatQueue_create fails.

Cheers,
Sam

diff --git a/common/c_cpp/src/c/queue.c b/common/c_cpp/src/c/queue.c
index d955b2b..c2ef48f 100644
--- a/common/c_cpp/src/c/queue.c
+++ b/common/c_cpp/src/c/queue.c
@@ -164,14 +164,27 @@ wombatQueue_destroy (wombatQueue queue)
result = WOMBAT_QUEUE_SEM_ERR;
}

- wInterlocked_destroy (&impl->mUnblocking);
-
wthread_mutex_destroy( &impl->mLock);
+
+ if (WOMBAT_QUEUE_OK == result)
+ {
+ return wombatQueue_deallocate(queue);
+ }
+ else
+ {
+ return result;
+ }
+}
+
+wombatQueueStatus
+wombatQueue_deallocate (wombatQueue queue)
+{
+ wombatQueueImpl *impl = (wombatQueueImpl*)queue;
+ wInterlocked_destroy (&impl->mUnblocking);
free (impl);
return WOMBAT_QUEUE_OK;
}

-
wombatQueueStatus
wombatQueue_setMaxSize (wombatQueue queue, unsigned int value)
{
diff --git a/common/c_cpp/src/c/wombat/queue.h
b/common/c_cpp/src/c/wombat/queue.h
index 79b75e9..caafddb 100644
--- a/common/c_cpp/src/c/wombat/queue.h
+++ b/common/c_cpp/src/c/wombat/queue.h
@@ -73,11 +73,24 @@ wombatQueue_create (wombatQueue queue, uint32_t
maxSize, uint32_t initialSize,
uint32_t growBySize);

/**
- * Destroy the Queue.
+ * Destroy the Queue, and free any memory associated
+ * with the Queue.
+ *
+ * wombatQueue_create() must have been successfully called
+ * before calling this method.
*/
COMMONExpDLL wombatQueueStatus
wombatQueue_destroy (wombatQueue queue);

+/**
+ * Free any memory associated with the Queue.
+ *
+ * Unless wombatQueue_create() failed, prefer calling
+ * wombatQueue_destroy().
+ */
+COMMONExpDLL wombatQueueStatus
+wombatQueue_deallocate(wombatQueue queue);
+
/**
* Set the maximum size of the queue. WOMBAT_QUEUE_MAX_SIZE is the maximum
* queue size permitted and the default value. This value should be a
multiple
diff --git a/mama/c_cpp/src/c/bridge/qpid/queue.c
b/mama/c_cpp/src/c/bridge/qpid/queue.c
index 5618b27..f12b0cf 100644
--- a/mama/c_cpp/src/c/bridge/qpid/queue.c
+++ b/mama/c_cpp/src/c/bridge/qpid/queue.c
@@ -137,7 +137,7 @@ qpidBridgeMamaQueue_create (queueBridge* queue,
mama_log (MAMA_LOG_LEVEL_ERROR,
"qpidBridgeMamaQueue_create (): "
"Failed to create underlying queue.");
- wombatQueue_destroy (impl->mQueue);
+ wombatQueue_deallocate (impl->mQueue);
free (impl);
return MAMA_STATUS_PLATFORM;
}
diff --git a/mama/c_cpp/src/c/conflation/manager.c
b/mama/c_cpp/src/c/conflation/manager.c
index db6b409..476db09 100644
--- a/mama/c_cpp/src/c/conflation/manager.c
+++ b/mama/c_cpp/src/c/conflation/manager.c
@@ -78,7 +78,12 @@ mamaConflationManager_create (mamaConflationManager mgr)
return MAMA_STATUS_NOMEM;

if (wombatQueue_create (impl->mMsgQueue, 0, 0, 0) != WOMBAT_QUEUE_OK)
+ {
+ wombatQueue_deallocate(impl->mMsgQueue);
+ impl->mMsgQueue = NULL;
+
return MAMA_STATUS_CONFLATE_ERROR;
+ }

status = mamaMsg_create (&impl->mMsg);


Damian Maguire <d.maguire@...>
 

Cheers for this Sam.

At a quick look the code all looks reasonable. Have you performed any
testing around the changes? It seems like a good candidate for some unit
tests - most likely within the queuetest.c code under
common/src/c_cpp/gunittest/c - just to ensure we have expected behaviour
under typical conditions.

Thanks,

D

On 29/10/14 19:33, Sam Wilson wrote:
Hey,

Here is a patch that adds wombatQueue_deallocate, which makes it
possible to reclaim resources if wombatQueue_create fails.

Cheers,
Sam

diff --git a/common/c_cpp/src/c/queue.c b/common/c_cpp/src/c/queue.c
index d955b2b..c2ef48f 100644
--- a/common/c_cpp/src/c/queue.c
+++ b/common/c_cpp/src/c/queue.c
@@ -164,14 +164,27 @@ wombatQueue_destroy (wombatQueue queue)
result = WOMBAT_QUEUE_SEM_ERR;
}

- wInterlocked_destroy (&impl->mUnblocking);
-
wthread_mutex_destroy( &impl->mLock);
+
+ if (WOMBAT_QUEUE_OK == result)
+ {
+ return wombatQueue_deallocate(queue);
+ }
+ else
+ {
+ return result;
+ }
+}
+
+wombatQueueStatus
+wombatQueue_deallocate (wombatQueue queue)
+{
+ wombatQueueImpl *impl = (wombatQueueImpl*)queue;
+ wInterlocked_destroy (&impl->mUnblocking);
free (impl);
return WOMBAT_QUEUE_OK;
}

-
wombatQueueStatus
wombatQueue_setMaxSize (wombatQueue queue, unsigned int value)
{
diff --git a/common/c_cpp/src/c/wombat/queue.h
b/common/c_cpp/src/c/wombat/queue.h
index 79b75e9..caafddb 100644
--- a/common/c_cpp/src/c/wombat/queue.h
+++ b/common/c_cpp/src/c/wombat/queue.h
@@ -73,11 +73,24 @@ wombatQueue_create (wombatQueue queue, uint32_t
maxSize, uint32_t initialSize,
uint32_t growBySize);

/**
- * Destroy the Queue.
+ * Destroy the Queue, and free any memory associated
+ * with the Queue.
+ *
+ * wombatQueue_create() must have been successfully called
+ * before calling this method.
*/
COMMONExpDLL wombatQueueStatus
wombatQueue_destroy (wombatQueue queue);

+/**
+ * Free any memory associated with the Queue.
+ *
+ * Unless wombatQueue_create() failed, prefer calling
+ * wombatQueue_destroy().
+ */
+COMMONExpDLL wombatQueueStatus
+wombatQueue_deallocate(wombatQueue queue);
+
/**
* Set the maximum size of the queue. WOMBAT_QUEUE_MAX_SIZE is the maximum
* queue size permitted and the default value. This value should be a
multiple
diff --git a/mama/c_cpp/src/c/bridge/qpid/queue.c
b/mama/c_cpp/src/c/bridge/qpid/queue.c
index 5618b27..f12b0cf 100644
--- a/mama/c_cpp/src/c/bridge/qpid/queue.c
+++ b/mama/c_cpp/src/c/bridge/qpid/queue.c
@@ -137,7 +137,7 @@ qpidBridgeMamaQueue_create (queueBridge* queue,
mama_log (MAMA_LOG_LEVEL_ERROR,
"qpidBridgeMamaQueue_create (): "
"Failed to create underlying queue.");
- wombatQueue_destroy (impl->mQueue);
+ wombatQueue_deallocate (impl->mQueue);
free (impl);
return MAMA_STATUS_PLATFORM;
}
diff --git a/mama/c_cpp/src/c/conflation/manager.c
b/mama/c_cpp/src/c/conflation/manager.c
index db6b409..476db09 100644
--- a/mama/c_cpp/src/c/conflation/manager.c
+++ b/mama/c_cpp/src/c/conflation/manager.c
@@ -78,7 +78,12 @@ mamaConflationManager_create (mamaConflationManager mgr)
return MAMA_STATUS_NOMEM;

if (wombatQueue_create (impl->mMsgQueue, 0, 0, 0) != WOMBAT_QUEUE_OK)
+ {
+ wombatQueue_deallocate(impl->mMsgQueue);
+ impl->mMsgQueue = NULL;
+
return MAMA_STATUS_CONFLATE_ERROR;
+ }

status = mamaMsg_create (&impl->mMsg);