Skip to content
Permalink
Browse files

Add macros for object free functions.

Most of the *Free() functions are pretty generic so add macros to make creating them as easy as possible.

Create a distinction between *Free() functions that the caller uses to free memory and callbacks that free third-party resources.  There are a number of cases where a driver needs to free resources but does not need a normal *Free() because it is handled by the interface.

Add common/object.h for macros that make object maintenance easier.  This pattern can also be used for many more object functions.
  • Loading branch information...
dwsteele committed May 3, 2019
1 parent 7ae9694 commit f1eea2312104ce7fa87119d6ef50a554b154e954
Showing with 684 additions and 910 deletions.
  1. +4 −0 doc/xml/release.xml
  2. +48 −48 src/Makefile.in
  3. +13 −23 src/common/compress/gzip/compress.c
  4. +13 −22 src/common/compress/gzip/decompress.c
  5. +9 −21 src/common/crypto/cipherBlock.c
  6. +13 −16 src/common/crypto/hash.c
  7. +38 −50 src/common/exec.c
  8. +4 −0 src/common/exec.h
  9. +4 −17 src/common/ini.c
  10. +3 −0 src/common/ini.h
  11. +2 −2 src/common/io/filter/buffer.c
  12. +3 −16 src/common/io/filter/filter.c
  13. +3 −0 src/common/io/filter/filter.h
  14. +3 −16 src/common/io/filter/group.c
  15. +3 −0 src/common/io/filter/group.h
  16. +2 −16 src/common/io/http/client.c
  17. +3 −0 src/common/io/http/client.h
  18. +3 −16 src/common/io/http/header.c
  19. +3 −0 src/common/io/http/header.h
  20. +3 −16 src/common/io/http/query.c
  21. +3 −0 src/common/io/http/query.h
  22. +3 −16 src/common/io/read.c
  23. +3 −0 src/common/io/read.h
  24. +13 −23 src/common/io/tls/client.c
  25. +3 −0 src/common/io/tls/client.h
  26. +3 −16 src/common/io/write.c
  27. +3 −0 src/common/io/write.h
  28. +1 −1 src/common/memContext.c
  29. +1 −6 src/common/memContext.h
  30. +75 −5 src/common/object.h
  31. +13 −22 src/common/regExp.c
  32. +3 −0 src/common/regExp.h
  33. +3 −16 src/common/type/buffer.c
  34. +3 −0 src/common/type/buffer.h
  35. +3 −16 src/common/type/keyValue.c
  36. +3 −0 src/common/type/keyValue.h
  37. +4 −17 src/common/type/list.c
  38. +3 −0 src/common/type/list.h
  39. +14 −23 src/common/type/xml.c
  40. +3 −0 src/common/type/xml.h
  41. +4 −17 src/common/wait.c
  42. +3 −0 src/common/wait.h
  43. +3 −16 src/info/info.c
  44. +3 −0 src/info/info.h
  45. +4 −17 src/info/infoArchive.c
  46. +3 −0 src/info/infoArchive.h
  47. +4 −17 src/info/infoBackup.c
  48. +3 −0 src/info/infoBackup.h
  49. +4 −18 src/info/infoPg.c
  50. +3 −0 src/info/infoPg.h
  51. +18 −28 src/protocol/client.c
  52. +3 −0 src/protocol/client.h
  53. +3 −16 src/protocol/command.c
  54. +3 −0 src/protocol/command.h
  55. +3 −16 src/protocol/parallel.c
  56. +3 −0 src/protocol/parallel.h
  57. +3 −16 src/protocol/parallelJob.c
  58. +3 −0 src/protocol/parallelJob.h
  59. +3 −16 src/protocol/server.c
  60. +3 −0 src/protocol/server.h
  61. +29 −41 src/storage/posix/read.c
  62. +62 −76 src/storage/posix/write.c
  63. +3 −16 src/storage/read.c
  64. +4 −1 src/storage/read.h
  65. +1 −1 src/storage/remote/read.c
  66. +2 −3 src/storage/remote/storage.c
  67. +39 −54 src/storage/remote/write.c
  68. +3 −3 src/storage/s3/storage.c
  69. +1 −1 src/storage/s3/write.c
  70. +3 −16 src/storage/storage.c
  71. +3 −0 src/storage/storage.h
  72. +3 −16 src/storage/write.c
  73. +4 −1 src/storage/write.h
  74. +7 −0 test/define.yaml
  75. +2 −8 test/src/module/common/compressGzipTest.c
  76. +9 −12 test/src/module/common/cryptoTest.c
  77. +1 −2 test/src/module/common/execTest.c
  78. +0 −1 test/src/module/common/iniTest.c
  79. +0 −3 test/src/module/common/ioHttpTest.c
  80. +0 −5 test/src/module/common/ioTest.c
  81. +0 −1 test/src/module/common/ioTlsTest.c
  82. +88 −0 test/src/module/common/objectTest.c
  83. +0 −1 test/src/module/common/regExpTest.c
  84. +0 −1 test/src/module/common/typeBufferTest.c
  85. +0 −1 test/src/module/common/typeKeyValueTest.c
  86. +0 −1 test/src/module/common/typeXmlTest.c
  87. +0 −1 test/src/module/common/waitTest.c
  88. +0 −1 test/src/module/info/infoArchiveTest.c
  89. +0 −1 test/src/module/info/infoBackupTest.c
  90. +0 −1 test/src/module/info/infoPgTest.c
  91. +0 −1 test/src/module/info/infoTest.c
  92. +0 −5 test/src/module/protocol/protocolTest.c
  93. +0 −5 test/src/module/storage/posixTest.c
  94. +3 −10 test/src/module/storage/remoteTest.c
  95. +1 −3 test/src/module/storage/s3Test.c
@@ -67,6 +67,10 @@
<p>Add <file>common/macro.h</file> for general-purpose macros.</p>
</release-item>

<release-item>
<p>Add macros for object free functions.</p>
</release-item>

<release-item>
<p>Various <code>MemContext</code> callback improvements.</p>
</release-item>

Large diffs are not rendered by default.

Oops, something went wrong.
@@ -23,6 +23,9 @@ Filter type constant
/***********************************************************************************************************************************
Object type
***********************************************************************************************************************************/
#define GZIP_COMPRESS_TYPE GzipCompress
#define GZIP_COMPRESS_PREFIX gzipCompress

typedef struct GzipCompress
{
MemContext *memContext; // Context to store data
@@ -54,6 +57,15 @@ Compression constants
***********************************************************************************************************************************/
#define MEM_LEVEL 9

/***********************************************************************************************************************************
Free deflate stream
***********************************************************************************************************************************/
OBJECT_DEFINE_FREE_RESOURCE_BEGIN(GZIP_COMPRESS, LOG, logLevelTrace)
{
deflateEnd(this->stream);
}
OBJECT_DEFINE_FREE_RESOURCE_END(LOG);

/***********************************************************************************************************************************
Compress data
***********************************************************************************************************************************/
@@ -146,28 +158,6 @@ gzipCompressInputSame(const THIS_VOID)
FUNCTION_TEST_RETURN(this->inputSame);
}

/***********************************************************************************************************************************
Free memory
***********************************************************************************************************************************/
static void
gzipCompressFree(GzipCompress *this)
{
FUNCTION_LOG_BEGIN(logLevelTrace);
FUNCTION_LOG_PARAM(GZIP_COMPRESS, this);
FUNCTION_LOG_END();

if (this != NULL)
{
deflateEnd(this->stream);
this->stream = NULL;

memContextCallbackClear(this->memContext);
memContextFree(this->memContext);
}

FUNCTION_LOG_RETURN_VOID();
}

/***********************************************************************************************************************************
New object
***********************************************************************************************************************************/
@@ -193,7 +183,7 @@ gzipCompressNew(int level, bool raw)
gzipError(deflateInit2(driver->stream, level, Z_DEFLATED, gzipWindowBits(raw), MEM_LEVEL, Z_DEFAULT_STRATEGY));

// Set free callback to ensure gzip context is freed
memContextCallbackSet(driver->memContext, (MemContextCallback)gzipCompressFree, driver);
memContextCallbackSet(driver->memContext, gzipCompressFreeResource, driver);

// Create filter interface
this = ioFilterNewP(
@@ -23,6 +23,9 @@ Filter type constant
/***********************************************************************************************************************************
Object type
***********************************************************************************************************************************/
#define GZIP_DECOMPRESS_TYPE GzipDecompress
#define GZIP_DECOMPRESS_PREFIX gzipDecompress

typedef struct GzipDecompress
{
MemContext *memContext; // Context to store data
@@ -49,6 +52,15 @@ gzipDecompressToLog(const GzipDecompress *this)
#define FUNCTION_LOG_GZIP_DECOMPRESS_FORMAT(value, buffer, bufferSize) \
FUNCTION_LOG_STRING_OBJECT_FORMAT(value, gzipDecompressToLog, buffer, bufferSize)

/***********************************************************************************************************************************
Free inflate stream
***********************************************************************************************************************************/
OBJECT_DEFINE_FREE_RESOURCE_BEGIN(GZIP_DECOMPRESS, LOG, logLevelTrace)
{
inflateEnd(this->stream);
}
OBJECT_DEFINE_FREE_RESOURCE_END(LOG);

/***********************************************************************************************************************************
Decompress data
***********************************************************************************************************************************/
@@ -125,27 +137,6 @@ gzipDecompressInputSame(const THIS_VOID)
FUNCTION_TEST_RETURN(this->inputSame);
}

/***********************************************************************************************************************************
Free memory
***********************************************************************************************************************************/
static void
gzipDecompressFree(GzipDecompress *this)
{
FUNCTION_LOG_BEGIN(logLevelTrace);
FUNCTION_LOG_PARAM(GZIP_DECOMPRESS, this);
FUNCTION_LOG_END();

if (this != NULL)
{
inflateEnd(this->stream);

memContextCallbackClear(this->memContext);
memContextFree(this->memContext);
}

FUNCTION_LOG_RETURN_VOID();
}

/***********************************************************************************************************************************
New object
***********************************************************************************************************************************/
@@ -169,7 +160,7 @@ gzipDecompressNew(bool raw)
gzipError(driver->result = inflateInit2(driver->stream, gzipWindowBits(raw)));

// Set free callback to ensure gzip context is freed
memContextCallbackSet(driver->memContext, (MemContextCallback)gzipDecompressFree, driver);
memContextCallbackSet(driver->memContext, gzipDecompressFreeResource, driver);

// Create filter interface
this = ioFilterNewP(
@@ -34,8 +34,11 @@ Header constants and sizes
#define CIPHER_BLOCK_HEADER_SIZE (CIPHER_BLOCK_MAGIC_SIZE + PKCS5_SALT_LEN)

/***********************************************************************************************************************************
Track state during block encrypt/decrypt
Object type
***********************************************************************************************************************************/
#define CIPHER_BLOCK_TYPE CipherBlock
#define CIPHER_BLOCK_PREFIX cipherBlock

typedef struct CipherBlock
{
MemContext *memContext; // Context to store data
@@ -71,28 +74,13 @@ cipherBlockToLog(const CipherBlock *this)
FUNCTION_LOG_STRING_OBJECT_FORMAT(value, cipherBlockToLog, buffer, bufferSize)

/***********************************************************************************************************************************
Free object
Free cipher context
***********************************************************************************************************************************/
static void
cipherBlockFree(CipherBlock *this)
OBJECT_DEFINE_FREE_RESOURCE_BEGIN(CIPHER_BLOCK, LOG, logLevelTrace)
{
FUNCTION_LOG_BEGIN(logLevelTrace);
FUNCTION_LOG_PARAM(CIPHER_BLOCK, this);
FUNCTION_LOG_END();

if (this != NULL)
{
// Free cipher context
if (this->cipherContext)
EVP_CIPHER_CTX_cleanup(this->cipherContext);

// Free mem context
memContextCallbackClear(this->memContext);
memContextFree(this->memContext);
}

FUNCTION_LOG_RETURN_VOID();
EVP_CIPHER_CTX_cleanup(this->cipherContext);
}
OBJECT_DEFINE_FREE_RESOURCE_END(LOG);

/***********************************************************************************************************************************
Determine how large the destination buffer should be
@@ -200,7 +188,7 @@ cipherBlockProcessBlock(CipherBlock *this, const unsigned char *source, size_t s
cryptoError(!(this->cipherContext = EVP_CIPHER_CTX_new()), "unable to create context");

// Set free callback to ensure cipher context is freed
memContextCallbackSet(this->memContext, (MemContextCallback)cipherBlockFree, this);
memContextCallbackSet(this->memContext, cipherBlockFreeResource, this);

// Initialize cipher
cryptoError(
@@ -32,6 +32,9 @@ STRING_EXTERN(HASH_TYPE_SHA256_STR, HASH_TYPE_SH
/***********************************************************************************************************************************
Object type
***********************************************************************************************************************************/
#define CRYPTO_HASH_TYPE CryptoHash
#define CRYPTO_HASH_PREFIX cryptoHash

typedef struct CryptoHash
{
MemContext *memContext; // Context to store data
@@ -48,6 +51,15 @@ Macros for function logging
#define FUNCTION_LOG_CRYPTO_HASH_FORMAT(value, buffer, bufferSize) \
objToLog(value, "CryptoHash", buffer, bufferSize)

/***********************************************************************************************************************************
Free hash context
***********************************************************************************************************************************/
OBJECT_DEFINE_FREE_RESOURCE_BEGIN(CRYPTO_HASH, LOG, logLevelTrace)
{
EVP_MD_CTX_destroy(this->hashContext);
}
OBJECT_DEFINE_FREE_RESOURCE_END(LOG);

/***********************************************************************************************************************************
Add message data to the hash from a Buffer
***********************************************************************************************************************************/
@@ -115,21 +127,6 @@ cryptoHashResult(THIS_VOID)
FUNCTION_LOG_RETURN(VARIANT, varNewStr(bufHex(cryptoHash(this))));
}

/***********************************************************************************************************************************
Free memory
***********************************************************************************************************************************/
static void
cryptoHashFreeCallback(CryptoHash *this)
{
FUNCTION_LOG_BEGIN(logLevelTrace);
FUNCTION_LOG_PARAM(CRYPTO_HASH, this);
FUNCTION_LOG_END();

EVP_MD_CTX_destroy(this->hashContext);

FUNCTION_LOG_RETURN_VOID();
}

/***********************************************************************************************************************************
New object
***********************************************************************************************************************************/
@@ -161,7 +158,7 @@ cryptoHashNew(const String *type)
cryptoError((driver->hashContext = EVP_MD_CTX_create()) == NULL, "unable to create hash context");

// Set free callback to ensure hash context is freed
memContextCallbackSet(driver->memContext, (MemContextCallback)cryptoHashFreeCallback, driver);
memContextCallbackSet(driver->memContext, cryptoHashFreeResource, driver);

// Initialize context
cryptoError(!EVP_DigestInit_ex(driver->hashContext, driver->hashType, NULL), "unable to initialize hash context");
@@ -45,6 +45,8 @@ struct Exec
IoWrite *ioWriteExec; // Wrapper for handle write interface
};

OBJECT_DEFINE_FREE(EXEC);

/***********************************************************************************************************************************
Macro to close file descriptors after dup2() in the child process
@@ -68,6 +70,41 @@ other code.
} \
while (0);

/***********************************************************************************************************************************
Free exec handles and ensure process is shut down
***********************************************************************************************************************************/
OBJECT_DEFINE_FREE_RESOURCE_BEGIN(EXEC, LOG, logLevelTrace)
{
// Close the io handles
close(this->handleRead);
close(this->handleWrite);
close(this->handleError);

// Wait for the child to exit. We don't really care how it exits as long as it does.
if (this->processId != 0)
{
MEM_CONTEXT_TEMP_BEGIN()
{
int processResult = 0;
Wait *wait = waitNew(this->timeout);

do
{
THROW_ON_SYS_ERROR(
(processResult = waitpid(this->processId, NULL, WNOHANG)) == -1, ExecuteError,
"unable to wait on child process");
}
while (processResult == 0 && waitMore(wait));

// If the process did not exit then error -- else we may end up with a collection of zombie processes
if (processResult == 0)
THROW_FMT(ExecuteError, "%s did not exit when expected", strPtr(this->name));
}
MEM_CONTEXT_TEMP_END();
}
}
OBJECT_DEFINE_FREE_RESOURCE_END(LOG);

/***********************************************************************************************************************************
New object
***********************************************************************************************************************************/
@@ -331,7 +368,7 @@ execOpen(Exec *this)
ioWriteOpen(this->ioWriteExec);

// Set a callback so the handles will get freed
memContextCallbackSet(this->memContext, (MemContextCallback)execFree, this);
memContextCallbackSet(this->memContext, execFreeResource, this);

FUNCTION_LOG_RETURN_VOID();
}
@@ -380,52 +417,3 @@ execMemContext(const Exec *this)

FUNCTION_TEST_RETURN(this->memContext);
}

/***********************************************************************************************************************************
Free the object
***********************************************************************************************************************************/
void
execFree(Exec *this)
{
FUNCTION_TEST_BEGIN();
FUNCTION_TEST_PARAM(EXEC, this);
FUNCTION_TEST_END();

if (this != NULL)
{
memContextCallbackClear(this->memContext);

// Close the io handles
close(this->handleRead);
close(this->handleWrite);
close(this->handleError);

// Wait for the child to exit. We don't really care how it exits as long as it does.
if (this->processId != 0)
{
MEM_CONTEXT_TEMP_BEGIN()
{
int processResult = 0;
Wait *wait = waitNew(this->timeout);

do
{
THROW_ON_SYS_ERROR(
(processResult = waitpid(this->processId, NULL, WNOHANG)) == -1, ExecuteError,
"unable to wait on child process");
}
while (processResult == 0 && waitMore(wait));

// If the process did not exit then error -- else we may end up with a collection of zombie processes
if (processResult == 0)
THROW_FMT(ExecuteError, "%s did not exit when expected", strPtr(this->name));
}
MEM_CONTEXT_TEMP_END();
}

// Free mem context
memContextFree(this->memContext);
}

FUNCTION_TEST_RETURN_VOID();
}
@@ -12,6 +12,9 @@ execution.
/***********************************************************************************************************************************
Object type
***********************************************************************************************************************************/
#define EXEC_TYPE Exec
#define EXEC_PREFIX exec

typedef struct Exec Exec;

#include "common/io/read.h"
@@ -27,6 +30,7 @@ Exec *execNew(const String *command, const StringList *param, const String *name
Functions
***********************************************************************************************************************************/
void execOpen(Exec *this);

/***********************************************************************************************************************************
Getters
***********************************************************************************************************************************/
Oops, something went wrong.

0 comments on commit f1eea23

Please sign in to comment.
You can’t perform that action at this time.