From b1d733e9a23645cc3289d27a9fec39cfce3c7981 Mon Sep 17 00:00:00 2001 From: Pietro Gagliardi Date: Mon, 11 May 2020 20:54:20 -0400 Subject: [PATCH] Reworked programmer error tests to limit what they catch. The cycle checking tests now correctly crash and burn because the programmer errors they throw are elsewhere. Now to actually hook up the cycle checking! Also added some notes to test/errors.c to explain what we're doing and also fixed the build. --- common/controls.c | 9 ++-- test/controls.c | 104 ++++++++++++++++++++++++---------------------- test/errors.c | 4 ++ test/initmain.c | 25 ++++++----- 4 files changed, 75 insertions(+), 67 deletions(-) diff --git a/common/controls.c b/common/controls.c index 0c6d3af5..b9e0d7ab 100644 --- a/common/controls.c +++ b/common/controls.c @@ -192,6 +192,7 @@ void uiControlFree(uiControl *c) static bool parentHasCycle(uiControl *c, uiControl *parent) { + // TODO remember if this is the correct way to use a local uiprivArray uiprivArray parents; size_t i; @@ -202,17 +203,17 @@ static bool parentHasCycle(uiControl *c, uiControl *parent) uiprivArrayInit(parents, uiControl *, 16, "uiControl parent list"); // add these now, as they are counted as part of any cycles - *((uiControl *) uiprivArrayAppend(parents, 1)) = c; - *((uiControl *) uiprivArrayAppend(parents, 1)) = parent; + *((uiControl **) uiprivArrayAppend(&parents, 1)) = c; + *((uiControl **) uiprivArrayAppend(&parents, 1)) = parent; for (c = parent->parent; c != NULL; c = c->parent) { // TODO this doesn't need to be sequential, but I don't imagine this list will ever be long enough to make it matter... yet for (i = 0; i < parents.len; i++) - if (c == uiprivArrayAt(parents, uiControl *, i)) { + if (c == *uiprivArrayAt(parents, uiControl *, i)) { uiprivArrayFree(parents); return true; } // new parent; mark it as visited - *((uiControl *) uiprivArrayAppend(parents, 1)) = c; + *((uiControl **) uiprivArrayAppend(&parents, 1)) = c; } uiprivArrayFree(parents); diff --git a/test/controls.c b/test/controls.c index 9f46e108..c2b06d58 100644 --- a/test/controls.c +++ b/test/controls.c @@ -216,37 +216,51 @@ Test(NullControlOSVtableIsProgrammerError) Test(CheckingNullControlIsProgrammerError) { + uint32_t ctrlType; void *ctx; + ctrlType = uiControlType(); ctx = beginCheckProgrammerError("uiCheckControlType(): invalid null pointer for uiControl"); - uiCheckControlType(NULL, uiControlType()); + uiCheckControlType(NULL, ctrlType); endCheckProgrammerError(ctx); } Test(CheckingNonControlIsProgrammerError) { + uiControl *c; + uint32_t ctrlType; void *ctx; + c = uiprivTestHookControlWithInvalidControlMarker(); + ctrlType = uiControlType(); ctx = beginCheckProgrammerError("uiCheckControlType(): object passed in not a uiControl"); - uiCheckControlType(uiprivTestHookControlWithInvalidControlMarker(), uiControlType()); + uiCheckControlType(c, ctrlType); endCheckProgrammerError(ctx); } Test(CheckingControlWithAnUnknownTypeIsProgrammerError) { + uiControl *c; + uint32_t ctrlType; void *ctx; + c = uiprivTestHookControlWithInvalidType(); + ctrlType = testControlType(); ctx = beginCheckProgrammerError("uiCheckControlType(): unknown uiControl type 0 found in uiControl (this is likely not a real uiControl or some data is corrupt)"); - uiCheckControlType(uiprivTestHookControlWithInvalidType(), testControlType()); + uiCheckControlType(c, ctrlType); endCheckProgrammerError(ctx); } Test(CheckingControlWithAnUnknownTypeIsProgrammerErrorEvenIfCheckingAgainstuiControlType) { + uiControl *c; + uint32_t ctrlType; void *ctx; + c = uiprivTestHookControlWithInvalidType(); + ctrlType = uiControlType(); ctx = beginCheckProgrammerError("uiCheckControlType(): unknown uiControl type 0 found in uiControl (this is likely not a real uiControl or some data is corrupt)"); - uiCheckControlType(uiprivTestHookControlWithInvalidType(), uiControlType()); + uiCheckControlType(c, ctrlType); endCheckProgrammerError(ctx); } @@ -255,31 +269,35 @@ Test(CheckingForUnknownControlTypeIsProgrammerError) uiControl *c; void *ctx; - ctx = beginCheckProgrammerError("uiCheckControlType(): unknown uiControl type 0 requested"); c = uiNewControl(testControlType(), NULL); + ctx = beginCheckProgrammerError("uiCheckControlType(): unknown uiControl type 0 requested"); uiCheckControlType(c, 0); - uiControlFree(c); endCheckProgrammerError(ctx); + uiControlFree(c); } Test(CheckControlTypeFailsCorrectly) { uiControl *c; + uint32_t ctrlType; void *ctx; - ctx = beginCheckProgrammerError("uiCheckControlType(): wrong uiControl type passed: got TestControl, want TestControl2"); c = uiNewControl(testControlType(), NULL); - uiCheckControlType(c, testControlType2()); - uiControlFree(c); + ctrlType = testControlType2(); + ctx = beginCheckProgrammerError("uiCheckControlType(): wrong uiControl type passed: got TestControl, want TestControl2"); + uiCheckControlType(c, ctrlType); endCheckProgrammerError(ctx); + uiControlFree(c); } Test(NewControlOfTypeControlIsProgrammerError) { + uint32_t ctrlType; void *ctx; + ctrlType = uiControlType(); ctx = beginCheckProgrammerError("uiNewControl(): uiControlType() passed in when specific control type needed"); - uiNewControl(uiControlType(), NULL); + uiNewControl(ctrlType, NULL); endCheckProgrammerError(ctx); } @@ -294,10 +312,12 @@ Test(NewControlOfUnknownTypeIsProgrammerError) Test(NewControlWithInvalidInitDataIsProgrammerError) { + uint32_t ctrlType; void *ctx; + ctrlType = testControlType(); ctx = beginCheckProgrammerError("uiNewControl(): invalid init data for TestControl"); - uiNewControl(testControlType(), testControlFailInit); + uiNewControl(ctrlType, testControlFailInit); endCheckProgrammerError(ctx); } @@ -310,27 +330,23 @@ Test(FreeingNullControlIsProgrammerError) endCheckProgrammerError(ctx); } -// TODO go back through all these tests and make the programmer error check as finely scoped as necessary Test(FreeingParentedControlIsProgrammerError) { uiControl *c, *d; void *ctx; - ctx = beginCheckProgrammerError("uiControlFree(): cannot be called on a control with has a parent"); - c = uiNewControl(testControlType(), NULL); d = uiNewControl(testControlType(), NULL); - // this should fail uiControlSetParent(c, d); + ctx = beginCheckProgrammerError("uiControlFree(): cannot be called on a control with has a parent"); uiControlFree(c); + endCheckProgrammerError(ctx); - // this should not fail; it's normal cleanup + // cleanup uiControlSetParent(c, NULL); uiControlFree(d); uiControlFree(c); - - endCheckProgrammerError(ctx); } Test(SetParentWithNullControlIsProgrammerError) @@ -347,11 +363,11 @@ Test(RemovingParentFromInitiallyParentlessControlIsProgrammerError) uiControl *c; void *ctx; - ctx = beginCheckProgrammerError("uiControlSetParent(): cannot set a control with no parent to have no parent"); c = uiNewControl(testControlType(), NULL); + ctx = beginCheckProgrammerError("uiControlSetParent(): cannot set a control with no parent to have no parent"); uiControlSetParent(c, NULL); - uiControlFree(c); endCheckProgrammerError(ctx); + uiControlFree(c); } Test(RemovingParentFromExplicitlyParentlessControlIsProgrammerError) @@ -359,15 +375,15 @@ Test(RemovingParentFromExplicitlyParentlessControlIsProgrammerError) uiControl *c, *d; void *ctx; - ctx = beginCheckProgrammerError("uiControlSetParent(): cannot set a control with no parent to have no parent"); c = uiNewControl(testControlType(), NULL); d = uiNewControl(testControlType(), NULL); uiControlSetParent(c, d); uiControlSetParent(c, NULL); + ctx = beginCheckProgrammerError("uiControlSetParent(): cannot set a control with no parent to have no parent"); uiControlSetParent(c, NULL); - uiControlFree(c); - uiControlFree(d); endCheckProgrammerError(ctx); + uiControlFree(d); + uiControlFree(c); } Test(ReparentingAlreadyParentedControlToDifferentParentIsProgrammerError) @@ -375,23 +391,20 @@ Test(ReparentingAlreadyParentedControlToDifferentParentIsProgrammerError) uiControl *c, *d, *e; void *ctx; - ctx = beginCheckProgrammerError("uiControlSetParent(): cannot set a control with a parent to have another parent"); - c = uiNewControl(testControlType(), NULL); d = uiNewControl(testControlType(), NULL); e = uiNewControl(testControlType(), NULL); - // this should fail uiControlSetParent(c, d); + ctx = beginCheckProgrammerError("uiControlSetParent(): cannot set a control with a parent to have another parent"); uiControlSetParent(c, e); + endCheckProgrammerError(ctx); - // this should not (cleanup) + // cleanup uiControlSetParent(c, NULL); uiControlFree(e); uiControlFree(d); uiControlFree(c); - - endCheckProgrammerError(ctx); } Test(ReparentingAlreadyParentedControlToSameParentIsProgrammerError) @@ -399,22 +412,18 @@ Test(ReparentingAlreadyParentedControlToSameParentIsProgrammerError) uiControl *c, *d; void *ctx; - ctx = beginCheckProgrammerError("uiControlSetParent(): cannot set a control with a parent to have another parent"); - c = uiNewControl(testControlType(), NULL); d = uiNewControl(testControlType(), NULL); - // this should fail uiControlSetParent(c, d); + ctx = beginCheckProgrammerError("uiControlSetParent(): cannot set a control with a parent to have another parent"); uiControlSetParent(c, d); + endCheckProgrammerError(ctx); - // this should not (cleanup) + // cleanup uiControlSetParent(c, NULL); - // TODO make sure all cleanups are in reverse order uiControlFree(d); uiControlFree(c); - - endCheckProgrammerError(ctx); } Test(ControlParentCyclesDisallowed_TwoControls) @@ -422,21 +431,19 @@ Test(ControlParentCyclesDisallowed_TwoControls) uiControl *c, *d; void *ctx; - ctx = beginCheckProgrammerError("TODO"); - c = uiNewControl(testControlType(), NULL); d = uiNewControl(testControlType(), NULL); - // this should fail uiControlSetParent(c, d); + ctx = beginCheckProgrammerError("TODO"); uiControlSetParent(d, c); + endCheckProgrammerError(ctx); - // this should not (cleanup) + // cleanup + // TODO reformat all the other tests to have clear init, test, and cleanup sections, and also maybe remove these "// cleanup" comments uiControlSetParent(c, NULL); uiControlFree(d); uiControlFree(c); - - endCheckProgrammerError(ctx); } Test(ControlParentCyclesDisallowed_ThreeControls) @@ -444,25 +451,22 @@ Test(ControlParentCyclesDisallowed_ThreeControls) uiControl *c, *d, *e; void *ctx; - ctx = beginCheckProgrammerError("TODO"); - c = uiNewControl(testControlType(), NULL); d = uiNewControl(testControlType(), NULL); e = uiNewControl(testControlType(), NULL); - // this should fail uiControlSetParent(c, d); uiControlSetParent(d, e); + ctx = beginCheckProgrammerError("TODO"); uiControlSetParent(e, c); + endCheckProgrammerError(ctx); - // this should not (cleanup) + // cleanup uiControlSetParent(d, NULL); uiControlSetParent(c, NULL); uiControlFree(e); uiControlFree(d); uiControlFree(c); - - endCheckProgrammerError(ctx); } Test(ControlCannotBeItsOwnParent) @@ -470,11 +474,11 @@ Test(ControlCannotBeItsOwnParent) uiControl *c; void *ctx; - ctx = beginCheckProgrammerError("TODO"); c = uiNewControl(testControlType(), NULL); + ctx = beginCheckProgrammerError("TODO"); uiControlSetParent(c, c); - uiControlFree(c); endCheckProgrammerError(ctx); + uiControlFree(c); } Test(GettingImplDataOfNullControlIsProgrammerError) diff --git a/test/errors.c b/test/errors.c index 076d169f..f6adb21c 100644 --- a/test/errors.c +++ b/test/errors.c @@ -5,6 +5,10 @@ // Do not put any test cases in this file; they will not be run. +// Notes on these functions: +// - Try to wrap them as tightly around the specific calls being tested as possible, to avoid accidentally catching something else. +// - I don't know if these are thread-safe yet (TODO potentially make them so so this first part can be made tighter). + struct checkProgrammerErrorParams { bool caught; char *msgGot; diff --git a/test/initmain.c b/test/initmain.c index a1c3218a..cfc3ab8a 100644 --- a/test/initmain.c +++ b/test/initmain.c @@ -8,8 +8,8 @@ static void testImplInitFailureFull(const char *file, long line) uiInitError err; void *ctx; - ctx = beginCheckProgrammerError(NULL); uiprivTestHookSetInitShouldFailArtificially(true); + ctx = beginCheckProgrammerError(NULL); memset(&err, 0, sizeof (uiInitError)); err.Size = sizeof (uiInitError); if (uiInit(NULL, &err)) @@ -149,28 +149,27 @@ Test(MainCalledTwiceIsProgrammerError) { void *ctx; - ctx = beginCheckProgrammerError("uiMain(): attempt to call more than once"); uiQueueMain(done, NULL); uiMain(); + ctx = beginCheckProgrammerError("uiMain(): attempt to call more than once"); uiMain(); endCheckProgrammerError(ctx); } static void mainAndQuit(void *data) { + void *ctx; + + ctx = beginCheckProgrammerError("uiMain(): attempt to call more than once"); uiMain(); + endCheckProgrammerError(ctx); uiQuit(); } Test(MainCalledRecursivelyIsProgrammerError) { - void *ctx; - - ctx = beginCheckProgrammerError("uiMain(): attempt to call more than once"); uiQueueMain(mainAndQuit, NULL); uiMain(); - uiMain(); - endCheckProgrammerError(ctx); } // largely redundant due to InitCorrectlyAfterInitializedSuccessfully, but include it anyway just to be safe @@ -179,9 +178,9 @@ Test(InitAfterMainIsProgrammerError) uiInitError err; void *ctx; - ctx = beginCheckProgrammerError("uiInit(): attempt to call more than once"); uiQueueMain(done, NULL); uiMain(); + ctx = beginCheckProgrammerError("uiInit(): attempt to call more than once"); memset(&err, 0, sizeof (uiInitError)); err.Size = sizeof (uiInitError); if (uiInit(NULL, &err)) @@ -200,27 +199,27 @@ Test(QuitBeforeMainIsProgrammerError) static void quitTwice(void *data) { + void *ctx; + uiQuit(); + ctx = beginCheckProgrammerError("uiQuit(): attempt to call more than once"); uiQuit(); + endCheckProgrammerError(ctx); } Test(QuitCalledTwiceIsProgrammerError) { - void *ctx; - - ctx = beginCheckProgrammerError("uiQuit(): attempt to call more than once"); uiQueueMain(quitTwice, NULL); uiMain(); - endCheckProgrammerError(ctx); } Test(QuitAfterMainIsProgrammerError) { void *ctx; - ctx = beginCheckProgrammerError("uiQuit(): attempt to call more than once"); uiQueueMain(done, NULL); uiMain(); + ctx = beginCheckProgrammerError("uiQuit(): attempt to call more than once"); uiQuit(); endCheckProgrammerError(ctx); }