Skip to content

Commit a40336d

Browse files
committed
Address CWE-404 faults in tests for pdalc_pipeline, pdalc_pointlayout, and pdalc_pointviewiterator
Addresses resource leaks identified by Coverity Scan. See: * http://cwe.mitre.org/data/definitions/404.html * https://scan4.coverity.com/doc/en/cov_checker_ref.html#static_checker_RESOURCE_LEAK
1 parent d62f1c7 commit a40336d

File tree

3 files changed

+67
-25
lines changed

3 files changed

+67
-25
lines changed

tests/pdal/test_pdalc_pipeline.c.in

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,9 @@
3232
#define __STDC_WANT_LIB_EXT1__ 1
3333
#endif
3434

35-
#include <stdio.h>
3635
#include <assert.h>
36+
#include <stdio.h> // for sscanf and sscanf_s
37+
#include <string.h> // for strncmp
3738

3839
#include <pdal/pdalc_pipeline.h>
3940

@@ -351,14 +352,12 @@ TEST testPDALValidatePipeline(void)
351352
PDALPipelinePtr pipeline = PDALCreatePipeline(gPipelineJson);
352353
ASSERT(pipeline);
353354
valid = PDALValidatePipeline(pipeline);
355+
PDALDisposePipeline(pipeline);
354356

355357
if (!valid)
356358
{
357-
PDALDisposePipeline(pipeline);
358-
pipeline = NULL;
359359
FAILm("PDALValidatePipeline returned false for a valid pipeline");
360360
}
361-
ASSERTm("Valid pipeline evaluated as invalid", valid);
362361

363362
PASS();
364363
}

tests/pdal/test_pdalc_pointlayout.c.in

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
*****************************************************************************/
2929

3030
#include <assert.h>
31+
#include <math.h> // for fabs
3132

3233
#include <pdal/pdalc.h>
3334

@@ -57,6 +58,8 @@ static void setup_test_pdalc_pointlayout(void *arg)
5758
gLayout = PDALGetPointViewLayout(gPointView);
5859
}
5960
}
61+
62+
PDALDisposePointViewIterator(views);
6063
}
6164
}
6265

@@ -76,9 +79,14 @@ TEST testPDALGetPointLayoutDimTypes(void)
7679
ASSERTm("PDALGetPointLayoutDimTypes should return non-NULL for a valid layout", types);
7780

7881
size_t size = PDALGetDimTypeListSize(types);
79-
ASSERTm("PDALGetDimTypeListSize should return more than zero for a valid list", size > 0);
8082

83+
// Dispose types list before assertion to avoid CWE-404
84+
// See http://cwe.mitre.org/data/definitions/404.html
85+
// See https://scan4.coverity.com/doc/en/cov_checker_ref.html#static_checker_RESOURCE_LEAK
8186
PDALDisposeDimTypeList(types);
87+
88+
ASSERTm("PDALGetDimTypeListSize should return more than zero for a valid list", size > 0);
89+
8290
PASS();
8391
}
8492

@@ -108,24 +116,38 @@ TEST testPDALFindDimType(void)
108116
for (size_t i = 0; i < size; ++i)
109117
{
110118
PDALDimType expected = PDALGetDimType(types, i);
111-
ASSERT(idUnknown != expected.id);
112-
ASSERT(typeNone != expected.type);
119+
120+
// Use check with FAIL instead of assertions to avoid CWE-404
121+
// See http://cwe.mitre.org/data/definitions/404.html
122+
// See https://scan4.coverity.com/doc/en/cov_checker_ref.html#static_checker_RESOURCE_LEAK
123+
124+
bool success = (idUnknown != expected.id);
125+
success &= (typeNone != expected.type);
113126

114127
char name[64];
115-
ASSERT(PDALGetDimTypeInterpretationName(expected, name, 64) > 0);
116-
ASSERT(PDALGetDimTypeIdName(expected, name, 64) > 0);
128+
success &= (PDALGetDimTypeInterpretationName(expected, name, 64) > 0);
129+
success &= (PDALGetDimTypeIdName(expected, name, 64) > 0);
117130

118131
actual = PDALFindDimType(NULL, name);
119-
ASSERT_EQ(idUnknown, actual.id);
120-
ASSERT_EQ(typeNone, actual.type);
121-
ASSERT_IN_RANGE(1.0, actual.scale, tolerance);
122-
ASSERT_IN_RANGE(0.0, actual.offset, tolerance);
132+
success &= (idUnknown == actual.id);
133+
success &= (typeNone == actual.type);
134+
135+
success &= (fabs(1.0 - actual.scale) < tolerance);
136+
success &= (fabs(actual.offset) < tolerance);
123137

124138
actual = PDALFindDimType(gLayout, name);
125-
ASSERT_EQ(expected.id, actual.id);
126-
ASSERT_EQ(expected.type, actual.type);
127-
ASSERT_IN_RANGE(expected.scale, actual.scale, tolerance);
128-
ASSERT_IN_RANGE(expected.offset, actual.offset, tolerance);
139+
success &= (expected.id == actual.id);
140+
success &= (expected.type == actual.type);
141+
142+
success &= (fabs(expected.scale - actual.scale) < tolerance);
143+
success &= (fabs(expected.offset - actual.offset) < tolerance);
144+
145+
if (!success)
146+
{
147+
PDALDisposeDimTypeList(types);
148+
types = NULL;
149+
FAILm("PDALGetPointLayoutDimTypes returned a list with an invalid element");
150+
}
129151
}
130152

131153
PDALDisposeDimTypeList(types);

tests/pdal/test_pdalc_pointviewiterator.c.in

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@
2727
* POSSIBILITY OF SUCH DAMAGE.
2828
*****************************************************************************/
2929

30-
#include <stdlib.h>
31-
#include <stdio.h>
3230
#include <assert.h>
3331

3432
#include <pdal/pdalc_pipeline.h>
@@ -87,8 +85,14 @@ TEST testPDALGetNextPointView(void)
8785
if (PDALHasNextPointView(gPointViewIterator))
8886
{
8987
PDALPointViewPtr anotherView = PDALGetNextPointView(gPointViewIterator);
90-
ASSERT(anotherView);
91-
ASSERT_FALSE(anotherView == view);
88+
89+
if (!anotherView || anotherView == view)
90+
{
91+
PDALDisposePointView(view);
92+
view = NULL;
93+
FAILm("PDALGetNextPointView returned the same view as the first view");
94+
}
95+
9296
PDALDisposePointView(anotherView);
9397
}
9498

@@ -109,17 +113,34 @@ TEST testPDALResetPointViewIterator(void)
109113
while (PDALHasNextPointView(gPointViewIterator))
110114
{
111115
PDALPointViewPtr anotherView = PDALGetNextPointView(gPointViewIterator);
112-
ASSERT(anotherView);
113-
ASSERT_FALSE(anotherView == view);
116+
117+
if (!anotherView || anotherView == view)
118+
{
119+
PDALDisposePointView(view);
120+
view = NULL;
121+
FAILm("PDALGetNextPointView returned the same view as the first view");
122+
}
123+
114124
PDALDisposePointView(anotherView);
115125
}
116126

117127
PDALDisposePointView(view);
118128

129+
// The next view should be null since we reached the end of the point view iteration
119130
view = PDALGetNextPointView(gPointViewIterator);
120-
ASSERT_EQ(NULL, view);
121-
PDALDisposePointView(view);
122131

132+
// Use check with FAIL instead of assertions to avoid CWE-404
133+
// See http://cwe.mitre.org/data/definitions/404.html
134+
// See https://scan4.coverity.com/doc/en/cov_checker_ref.html#static_checker_RESOURCE_LEAK
135+
136+
if (view)
137+
{
138+
PDALDisposePointView(view);
139+
view = NULL;
140+
FAILm("PDALGetNextPointView returned a non-null view immediately after PDALHasNextPointView returned false");
141+
}
142+
143+
// The next view should be non-null since we reset the point view iterator
123144
PDALResetPointViewIterator(gPointViewIterator);
124145

125146
view = PDALGetNextPointView(gPointViewIterator);

0 commit comments

Comments
 (0)