Skip to content

Commit 38220af

Browse files
robohackrobohack
committed
finally fix the old memory leak in yajl_tree_parse()
- use wrapped malloc() et al wrappers consistently - update example/parse_config.c to do memory leak detection - add a regression test using example/parse_config Several issues in lloyd/yajl complained about this leak, and comments in lloyd/yajl#102 showed a mostly correct fix though none of these issues mentioned or actually fixed the directly related error reporting problem. Fixes lloyd/yajl#102, fixes lloyd/yajl#113, fixes lloyd/yajl#168, fixes lloyd/yajl#191, fixes lloyd/yajl#223, fixes lloyd/yajl#250. Also fixes lloy/yajl#185.
1 parent 270153b commit 38220af

File tree

6 files changed

+220
-81
lines changed

6 files changed

+220
-81
lines changed

‎example/Makefile

+10-3
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,19 @@ MAN = # empty
99
# don't install
1010
proginstall:
1111

12+
# xxx hmmm.... stdout and stderr are reversed
13+
#
14+
T1 = [ $$(expr "$$(${.OBJDIR}/${PROG} < ${.CURDIR}/sample.config 2>&1)" : "memory leaks:.0.Logging/timeFormat: utc") -eq 39 ]
15+
T2 = [ $$(expr "$$(echo '{broken:' | ${.OBJDIR}/${PROG} 2>&1)" : "tree_parse_error: lexical error: invalid char in json text.*memory leaks:.0") -eq 165 ]
16+
1217
regress:
1318
.if defined(USE_ASAN)
14-
if [ -x /usr/sbin/paxctl ]; then /usr/sbin/paxctl +a ./parse_config; fi
15-
ulimit -v unlimited && [ "$$(${.OBJDIR}/${PROG} < ${.CURDIR}/sample.config)" = "Logging/timeFormat: utc" ]
19+
if [ -x /usr/sbin/paxctl ]; then /usr/sbin/paxctl +a ${.OBJDIR}/${PROG}; fi
20+
ulimit -v unlimited && ${T1}
21+
ulimit -v unlimited && ${T2}
1622
.else
17-
[ "$$(${.OBJDIR}/${PROG} < ${.CURDIR}/sample.config)" = "Logging/timeFormat: utc" ]
23+
${T1}
24+
${T2}
1825
.endif
1926

2027
.include <bsd.prog.mk>

‎example/parse_config.c

+101-18
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,78 @@
1515
*/
1616

1717
#include <stdio.h>
18+
#include <stdbool.h>
19+
#include <stdlib.h>
1820
#include <string.h>
1921

22+
#include <assert.h>
23+
2024
#include "yajl/yajl_tree.h"
2125

22-
static unsigned char fileData[65536];
26+
/* context storage for memory debugging routines */
27+
typedef struct
28+
{
29+
bool do_printfs;
30+
unsigned int numFrees;
31+
unsigned int numMallocs;
32+
/* XXX: we really need a hash table here with per-allocation
33+
* information to find any missing free() calls */
34+
} yajlTestMemoryContext;
35+
36+
/* cast void * into context */
37+
#define TEST_CTX(vptr) ((yajlTestMemoryContext *) (vptr))
38+
39+
static void
40+
yajlTestFree(void *ctx,
41+
void *ptr)
42+
{
43+
assert(ptr != NULL);
44+
TEST_CTX(ctx)->numFrees++;
45+
if (TEST_CTX(ctx)->do_printfs) {
46+
fprintf(stderr, "yfree: %p\n", ptr);
47+
}
48+
free(ptr);
49+
}
50+
51+
static void *
52+
yajlTestMalloc(void *ctx,
53+
size_t sz)
54+
{
55+
void *rv = NULL;
56+
57+
assert(sz != 0);
58+
TEST_CTX(ctx)->numMallocs++;
59+
rv = malloc(sz);
60+
assert(rv != NULL);
61+
if (TEST_CTX(ctx)->do_printfs) {
62+
fprintf(stderr, "yalloc: %p of %ju\n", rv, sz);
63+
}
64+
return rv;
65+
}
66+
67+
static void *
68+
yajlTestRealloc(void *ctx,
69+
void *ptr,
70+
size_t sz)
71+
{
72+
void *rv = NULL;
73+
74+
if (ptr == NULL) {
75+
assert(sz != 0);
76+
TEST_CTX(ctx)->numMallocs++;
77+
} else if (sz == 0) {
78+
TEST_CTX(ctx)->numFrees++;
79+
}
80+
rv = realloc(ptr, sz);
81+
assert(rv != NULL);
82+
if (TEST_CTX(ctx)->do_printfs) {
83+
fprintf(stderr, "yrealloc: %p -> %p of %ju\n", ptr, rv, sz);
84+
}
85+
return rv;
86+
}
87+
88+
89+
static unsigned char fileData[65536]; /* xxx: allocate to size of file, error if stdin can't be stat()ed? */
2390

2491
int
2592
main(void)
@@ -28,35 +95,49 @@ main(void)
2895
yajl_val node;
2996
char errbuf[1024];
3097

31-
/* NUL plug the buffers */
32-
fileData[0] = '\0';
33-
errbuf[0] = '\0';
98+
/* memory allocation debugging: allocate a structure which holds
99+
* allocation routines */
100+
yajl_alloc_funcs allocFuncs = {
101+
yajlTestMalloc,
102+
yajlTestRealloc,
103+
yajlTestFree,
104+
(void *) NULL
105+
};
106+
107+
/* memory allocation debugging: allocate a structure which collects
108+
* statistics */
109+
yajlTestMemoryContext memCtx;
110+
111+
memCtx.do_printfs = false; /* xxx set from a command option */
112+
memCtx.numMallocs = 0;
113+
memCtx.numFrees = 0;
114+
115+
allocFuncs.ctx = (void *) &memCtx;
116+
yajl_tree_parse_afs = &allocFuncs;
34117

35118
/* read the entire config file */
36119
rd = fread((void *) fileData, (size_t) 1, sizeof(fileData) - 1, stdin);
37120

38121
/* file read error handling */
39-
if (rd == 0 && !feof(stdin)) {
40-
fprintf(stderr, "error encountered on file read\n");
41-
return 1;
42-
} else if (rd >= sizeof(fileData) - 1) {
122+
if ((rd == 0 && !feof(stdin)) || ferror(stdin)) {
123+
perror("error encountered on file read");
124+
exit(1);
125+
} else if (!feof(stdin)) {
43126
fprintf(stderr, "config file too big\n");
44-
return 1;
127+
exit(1);
45128
}
129+
fileData[rd] = '\0';
46130

47131
/* we have the whole config file in memory. let's parse it ... */
48132
node = yajl_tree_parse((const char *) fileData, errbuf, sizeof(errbuf));
49133

50134
/* parse error handling */
51135
if (node == NULL) {
52-
fprintf(stderr, "parse_error: ");
53-
if (strlen(errbuf)) {
54-
fprintf(stderr, "%s", errbuf);
55-
} else {
56-
fprintf(stderr, "unknown error");
57-
}
58-
fprintf(stderr, "\n");
59-
return 1;
136+
assert(errbuf != NULL);
137+
fprintf(stderr, "tree_parse_error: %s\n", errbuf);
138+
fprintf(stderr, "memory leaks:\t%u\n", memCtx.numMallocs - memCtx.numFrees);
139+
140+
exit(1);
60141
}
61142

62143
/* ... and extract a nested value from the config file */
@@ -74,5 +155,7 @@ main(void)
74155

75156
yajl_tree_free(node);
76157

77-
return 0;
158+
fprintf(stderr, "memory leaks:\t%u\n", memCtx.numMallocs - memCtx.numFrees);
159+
160+
exit(memCtx.numMallocs - memCtx.numFrees ? 1 : 0);
78161
}

‎src/Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ install: includes
166166
incinstall: .PHONY
167167
.endif
168168

169-
.if ${TARGET_OSNAME} == "Linux"
169+
.if defined(TARGET_OSNAME) && ${TARGET_OSNAME} == "Linux"
170170
# XXX stupid GNU BinUtils LD changed its command line syntax recently,
171171
# apparently without concern for backward compatability.
172172
#

‎src/yajl/yajl_tree.h

+3
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@
3737
extern "C" {
3838
#endif
3939

40+
/*+ an optional hook to allow use of custom yajl_alloc_funcs with yajl_tree_parse() +*/
41+
extern yajl_alloc_funcs *yajl_tree_parse_afs;
42+
4043
/*+ possible data types that a yajl_val_s can hold +*/
4144
typedef enum {
4245
yajl_t_string = 1,

‎src/yajl_parser.c

+25-18
Original file line numberDiff line numberDiff line change
@@ -158,27 +158,34 @@ yajl_status
158158
yajl_do_finish(yajl_handle hand)
159159
{
160160
yajl_status stat;
161-
stat = yajl_do_parse(hand,(const unsigned char *) " ",(size_t) 1);
161+
size_t offset = hand->bytesConsumed;
162162

163-
if (stat != yajl_status_ok) return stat;
163+
stat = yajl_do_parse(hand, (const unsigned char *) " ", (size_t) 1);
164+
hand->bytesConsumed = offset;
164165

165-
switch(yajl_bs_current(hand->stateStack))
166-
{
167-
case yajl_state_parse_error:
168-
case yajl_state_lexical_error:
169-
return yajl_status_error;
170-
case yajl_state_got_value:
171-
case yajl_state_parse_complete:
172-
return yajl_status_ok;
173-
default:
174-
if (!(hand->flags & yajl_allow_partial_values))
175-
{
176-
yajl_bs_set(hand->stateStack, yajl_state_parse_error);
177-
hand->parseError = "premature EOF";
178-
return yajl_status_error;
179-
}
180-
return yajl_status_ok;
166+
if (stat != yajl_status_ok) {
167+
return stat;
168+
}
169+
170+
switch (yajl_bs_current(hand->stateStack)) {
171+
case yajl_state_parse_error:
172+
case yajl_state_lexical_error:
173+
return yajl_status_error;
174+
175+
case yajl_state_got_value:
176+
case yajl_state_parse_complete:
177+
return yajl_status_ok;
178+
179+
default:
180+
break;
181+
}
182+
if (!(hand->flags & yajl_allow_partial_values)) {
183+
yajl_bs_set(hand->stateStack, yajl_state_parse_error);
184+
hand->parseError = "premature EOF";
185+
186+
return yajl_status_error;
181187
}
188+
return yajl_status_ok;
182189
}
183190

184191
yajl_status

0 commit comments

Comments
 (0)