Skip to content
Permalink
Browse files

Improve type safety of interfaces and drivers.

The function pointer casting used when creating drivers made changing interfaces difficult and led to slightly divergent driver implementations.  Unit testing caught production-level errors but there were a lot of small issues and the process was harder than it should have been.

Use void pointers instead so that no casts are required.  Introduce the THIS_VOID and THIS() macros to make dealing with void pointers a little safer.

Since we don't want to expose void pointers in header files, driver functions have been removed from the headers and the various driver objects return their interface type.  This cuts down on accessor methods and the vast majority of those functions were not being used.  Move functions that are still required to .intern.h.

Remove the special "C" crypto functions that were used in libc and instead use the standard interface.
  • Loading branch information...
dwsteele committed May 2, 2019
1 parent 28359ee commit 8c712d89ebe10e3d318952a0b4db8cd09f8a580c
Showing with 2,545 additions and 4,063 deletions.
  1. +8 −0 doc/xml/release.xml
  2. +38 −6 libc/xs/crypto/cipherBlock.xs
  3. +3 −1 libc/xs/crypto/cipherBlock.xsh
  4. +6 −5 libc/xs/crypto/hash.xs
  5. +2 −1 libc/xs/crypto/hash.xsh
  6. +3 −1 libc/xs/storage/storage.xs
  7. +31 −31 src/Makefile.in
  8. +2 −4 src/command/archive/get/file.c
  9. +3 −6 src/command/archive/push/file.c
  10. +2 −2 src/command/local/local.c
  11. +2 −2 src/command/remote/remote.c
  12. +68 −74 src/common/compress/gzip/compress.c
  13. +1 −34 src/common/compress/gzip/compress.h
  14. +59 −64 src/common/compress/gzip/decompress.c
  15. +1 −34 src/common/compress/gzip/decompress.h
  16. +106 −132 src/common/crypto/cipherBlock.c
  17. +1 −43 src/common/crypto/cipherBlock.h
  18. +69 −161 src/common/crypto/hash.c
  19. +1 −36 src/common/crypto/hash.h
  20. +111 −104 src/common/exec.c
  21. +0 −6 src/common/exec.h
  22. +32 −69 src/common/io/bufferRead.c
  23. +1 −31 src/common/io/bufferRead.h
  24. +28 −67 src/common/io/bufferWrite.c
  25. +1 −30 src/common/io/bufferWrite.h
  26. +32 −57 src/common/io/filter/buffer.c
  27. +1 −32 src/common/io/filter/buffer.h
  28. +47 −1 src/common/io/filter/filter.c
  29. +6 −1 src/common/io/filter/filter.h
  30. +7 −11 src/common/io/filter/filter.intern.h
  31. +1 −1 src/common/io/filter/group.c
  32. +36 −69 src/common/io/filter/size.c
  33. +1 −33 src/common/io/filter/size.h
  34. +38 −75 src/common/io/handleRead.c
  35. +1 −32 src/common/io/handleRead.h
  36. +32 −70 src/common/io/handleWrite.c
  37. +1 −31 src/common/io/handleWrite.h
  38. +8 −4 src/common/io/http/client.c
  39. +30 −0 src/common/io/read.c
  40. +15 −11 src/common/io/read.intern.h
  41. +144 −138 src/common/io/tls/client.c
  42. +0 −3 src/common/io/tls/client.h
  43. +30 −0 src/common/io/write.c
  44. +1 −1 src/common/io/write.h
  45. +10 −9 src/common/io/write.intern.h
  46. +20 −0 src/common/object.h
  47. +19 −20 src/info/info.c
  48. +47 −12 src/perl/libc.auto.c
  49. +5 −122 src/storage/driver/cifs/storage.c
  50. +1 −39 src/storage/driver/cifs/storage.h
  51. +104 −145 src/storage/driver/posix/fileRead.c
  52. +1 −38 src/storage/driver/posix/fileRead.h
  53. +169 −300 src/storage/driver/posix/fileWrite.c
  54. +2 −44 src/storage/driver/posix/fileWrite.h
  55. +119 −111 src/storage/driver/posix/storage.c
  56. +1 −45 src/storage/driver/posix/storage.h
  57. +31 −0 src/storage/driver/posix/storage.intern.h
  58. +63 −108 src/storage/driver/remote/fileRead.c
  59. +2 −37 src/storage/driver/remote/fileRead.h
  60. +130 −261 src/storage/driver/remote/fileWrite.c
  61. +2 −44 src/storage/driver/remote/fileWrite.h
  62. +72 −80 src/storage/driver/remote/storage.c
  63. +2 −42 src/storage/driver/remote/storage.h
  64. +22 −0 src/storage/driver/remote/storage.intern.h
  65. +64 −108 src/storage/driver/s3/fileRead.c
  66. +2 −37 src/storage/driver/s3/fileRead.h
  67. +72 −213 src/storage/driver/s3/fileWrite.c
  68. +2 −44 src/storage/driver/s3/fileWrite.h
  69. +103 −107 src/storage/driver/s3/storage.c
  70. +2 −60 src/storage/driver/s3/storage.h
  71. +46 −0 src/storage/driver/s3/storage.intern.h
  72. +23 −18 src/storage/fileRead.c
  73. +0 −1 src/storage/fileRead.h
  74. +8 −16 src/storage/fileRead.intern.h
  75. +26 −26 src/storage/fileWrite.c
  76. +1 −4 src/storage/fileWrite.h
  77. +18 −26 src/storage/fileWrite.intern.h
  78. +28 −40 src/storage/helper.c
  79. +1 −1 src/storage/storage.c
  80. +15 −27 src/storage/storage.intern.h
  81. +1 −0 test/define.yaml
  82. +2 −2 test/lib/pgBackRestTest/Module/Storage/StorageFilterCipherBlockPerlTest.pm
  83. +1 −1 test/src/common/harnessInfo.c
  84. +2 −2 test/src/module/command/archiveCommonTest.c
  85. +7 −10 test/src/module/command/archiveGetTest.c
  86. +9 −11 test/src/module/command/archivePushTest.c
  87. +2 −2 test/src/module/command/controlTest.c
  88. +2 −2 test/src/module/command/helpTest.c
  89. +2 −2 test/src/module/command/infoTest.c
  90. +2 −2 test/src/module/command/localTest.c
  91. +10 −10 test/src/module/command/remoteTest.c
  92. +11 −9 test/src/module/common/compressGzipTest.c
  93. +49 −49 test/src/module/common/cryptoTest.c
  94. +2 −2 test/src/module/common/iniTest.c
  95. +119 −162 test/src/module/common/ioTest.c
  96. +2 −2 test/src/module/common/lockTest.c
  97. +4 −4 test/src/module/config/protocolTest.c
  98. +3 −4 test/src/module/info/infoTest.c
  99. +2 −2 test/src/module/postgres/interfaceTest.c
  100. +20 −20 test/src/module/protocol/protocolTest.c
  101. +0 −5 test/src/module/storage/cifsTest.c
  102. +27 −35 test/src/module/storage/posixTest.c
  103. +9 −9 test/src/module/storage/remoteTest.c
  104. +14 −17 test/src/module/storage/s3Test.c
@@ -55,6 +55,14 @@
<p>Don't append <code>strerror()</code> to error message when <code>errno</code> is 0.</p>
</release-item>

<release-item>
<release-item-contributor-list>
<release-item-reviewer id="cynthia.shang"/>
</release-item-contributor-list>

<p>Improve type safety of interfaces and drivers.</p>
</release-item>

<release-item>
<p>Various <code>Buffer</code> improvements.</p>
</release-item>
@@ -18,17 +18,20 @@ new(class, mode, type, key, keySize, digest = NULL)
CODE:
RETVAL = NULL;

CHECK(type != NULL);
CHECK(key != NULL);
CHECK(keySize != 0);

// Not much point to this but it keeps the var from being unused
if (strcmp(class, PACKAGE_NAME_LIBC "::Cipher::Block") != 0)
croak("unexpected class name '%s'", class);

MEM_CONTEXT_XS_NEW_BEGIN("cipherBlockXs")
{
RETVAL = memNew(sizeof(CipherBlockXs));

RETVAL->memContext = MEM_COMTEXT_XS();

RETVAL->pxPayload = cipherBlockNewC(mode, type, key, keySize, digest);
RETVAL->pxPayload = cipherBlockNew(mode, cipherType(STR(type)), BUF(key, keySize), digest == NULL ? NULL : STR(digest));
}
MEM_CONTEXT_XS_NEW_END();
OUTPUT:
@@ -47,10 +50,27 @@ CODE:
STRLEN tSize;
const unsigned char *sourcePtr = (const unsigned char *)SvPV(source, tSize);

RETVAL = NEWSV(0, cipherBlockProcessSizeC(self->pxPayload, tSize));
RETVAL = NEWSV(0, ioBufferSize());
SvPOK_only(RETVAL);

SvCUR_set(RETVAL, cipherBlockProcessC(self->pxPayload, sourcePtr, tSize, (unsigned char *)SvPV_nolen(RETVAL)));
if (tSize > 0)
{
size_t outBufferUsed = 0;

do
{
SvGROW(RETVAL, outBufferUsed + ioBufferSize());
Buffer *outBuffer = bufNewUseC((unsigned char *)SvPV_nolen(RETVAL) + outBufferUsed, ioBufferSize());

ioFilterProcessInOut(self->pxPayload, BUF(sourcePtr, tSize), outBuffer);
outBufferUsed += bufUsed(outBuffer);
}
while (ioFilterInputSame(self->pxPayload));

SvCUR_set(RETVAL, outBufferUsed);
}
else
SvCUR_set(RETVAL, 0);
}
MEM_CONTEXT_XS_END();
OUTPUT:
@@ -65,10 +85,22 @@ CODE:

MEM_CONTEXT_XS_BEGIN(self->memContext)
{
RETVAL = NEWSV(0, cipherBlockProcessSizeC(self->pxPayload, 0));
RETVAL = NEWSV(0, ioBufferSize());
SvPOK_only(RETVAL);

SvCUR_set(RETVAL, cipherBlockFlushC(self->pxPayload, (unsigned char *)SvPV_nolen(RETVAL)));
size_t outBufferUsed = 0;

do
{
SvGROW(RETVAL, outBufferUsed + ioBufferSize());
Buffer *outBuffer = bufNewUseC((unsigned char *)SvPV_nolen(RETVAL) + outBufferUsed, ioBufferSize());

ioFilterProcessInOut(self->pxPayload, NULL, outBuffer);
outBufferUsed += bufUsed(outBuffer);
}
while (!ioFilterDone(self->pxPayload));

SvCUR_set(RETVAL, outBufferUsed);
}
MEM_CONTEXT_XS_END();
OUTPUT:
@@ -1,7 +1,9 @@
/***********************************************************************************************************************************
Block Cipher XS Header
***********************************************************************************************************************************/
#include "common/assert.h"
#include "common/crypto/cipherBlock.h"
#include "common/io/io.h"
#include "common/memContext.h"

// Encrypt/decrypt modes
@@ -11,5 +13,5 @@ Block Cipher XS Header
typedef struct CipherBlockXs
{
MemContext *memContext;
CipherBlock *pxPayload;
IoFilter *pxPayload;
} CipherBlockXs, *pgBackRest__LibC__Cipher__Block;
@@ -36,9 +36,10 @@ CODE:
MEM_CONTEXT_XS_TEMP_BEGIN()
{
STRLEN messageSize;
const unsigned char *messagePtr = (const unsigned char *)SvPV(message, messageSize);
const void *messagePtr = SvPV(message, messageSize);

cryptoHashProcessC(self->pxPayload, messagePtr, messageSize);
if (messageSize > 0)
ioFilterProcessIn(self->pxPayload, BUF(messagePtr, messageSize));
}
MEM_CONTEXT_XS_TEMP_END();

@@ -51,7 +52,7 @@ CODE:

MEM_CONTEXT_XS_TEMP_BEGIN()
{
String *hash = bufHex(cryptoHash(self->pxPayload));
const String *hash = varStr(ioFilterResult(self->pxPayload));

RETVAL = newSV(strSize(hash));
SvPOK_only(RETVAL);
@@ -82,9 +83,9 @@ CODE:
MEM_CONTEXT_XS_TEMP_BEGIN()
{
STRLEN messageSize;
const unsigned char *messagePtr = (const unsigned char *)SvPV(message, messageSize);
const void *messagePtr = SvPV(message, messageSize);

String *hash = bufHex(cryptoHashOneC(strNew(type), messagePtr, messageSize));
String *hash = bufHex(cryptoHashOne(strNew(type), BUF(messagePtr, messageSize)));

RETVAL = newSV(strSize(hash));
SvPOK_only(RETVAL);
@@ -2,10 +2,11 @@
Cryptographic Hashes XS Header
***********************************************************************************************************************************/
#include "common/crypto/hash.h"
#include "common/io/filter/filter.intern.h"
#include "common/memContext.h"

typedef struct CryptoHashXs
{
MemContext *memContext;
CryptoHash *pxPayload;
IoFilter *pxPayload;
} CryptoHashXs, *pgBackRest__LibC__Crypto__Hash;
@@ -13,6 +13,8 @@ storageDriverPosixPathRemove(path, errorOnMissing, recurse)
CODE:
MEM_CONTEXT_XS_TEMP_BEGIN()
{
storageDriverPosixPathRemove(storageDriverPosixNew(strNew("/"), 0640, 750, true, NULL), strNew(path), errorOnMissing, recurse);
storagePathRemoveP(
storageDriverPosixNew(strNew("/"), 0640, 750, true, NULL), strNew(path), .errorOnMissing = errorOnMissing,
.recurse = recurse);
}
MEM_CONTEXT_XS_TEMP_END();
Oops, something went wrong.

0 comments on commit 8c712d8

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