Skip to content

Commit 0ea9464

Browse files
ZigzagAKZigzagAK
authored
Fix possible memory and file descriptors leaks (#2258)
- Used postgres memory allocation functions instead of standard ones. - Wrapped main loop of csv loader in PG_TRY block for better error handling.
1 parent fa9973a commit 0ea9464

File tree

5 files changed

+135
-117
lines changed

5 files changed

+135
-117
lines changed

‎src/backend/utils/adt/age_global_graph.c‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1237,12 +1237,10 @@ Datum age_delete_global_graphs(PG_FUNCTION_ARGS)
12371237
{
12381238
char *graph_name = NULL;
12391239

1240-
graph_name = strndup(agtv_temp->val.string.val,
1241-
agtv_temp->val.string.len);
1240+
graph_name = pnstrdup(agtv_temp->val.string.val,
1241+
agtv_temp->val.string.len);
12421242

12431243
success = delete_specific_GRAPH_global_contexts(graph_name);
1244-
1245-
free(graph_name);
12461244
}
12471245
else
12481246
{

‎src/backend/utils/adt/agtype.c‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,17 @@ static agtype_value *agtype_build_map_as_agtype_value(FunctionCallInfo fcinfo);
181181
agtype_value *agtype_composite_to_agtype_value_binary(agtype *a);
182182
static agtype_value *tostring_helper(Datum arg, Oid type, char *msghdr);
183183

184+
185+
void *repalloc_check(void *ptr, size_t len)
186+
{
187+
if (ptr != NULL)
188+
{
189+
return repalloc(ptr, len);
190+
}
191+
192+
return palloc(len);
193+
}
194+
184195
/*
185196
* Due to how pfree can be implemented, it may not check for a passed NULL. This
186197
* wrapper does just that, it will only call pfree is the pointer passed is not
@@ -5580,7 +5591,7 @@ static char *get_label_name(const char *graph_name, graphid element_graphid)
55805591
result = NameStr(*DatumGetName(heap_getattr(tuple, Anum_ag_label_name,
55815592
tupdesc, &column_is_null)));
55825593
/* duplicate it */
5583-
result = strdup(result);
5594+
result = pstrdup(result);
55845595

55855596
/* end the scan and close the relation */
55865597
systable_endscan(scan_desc);
@@ -5673,8 +5684,8 @@ Datum age_startnode(PG_FUNCTION_ARGS)
56735684
Assert(AGT_ROOT_IS_SCALAR(agt_arg));
56745685
agtv_object = get_ith_agtype_value_from_container(&agt_arg->root, 0);
56755686
Assert(agtv_object->type == AGTV_STRING);
5676-
graph_name = strndup(agtv_object->val.string.val,
5677-
agtv_object->val.string.len);
5687+
graph_name = pnstrdup(agtv_object->val.string.val,
5688+
agtv_object->val.string.len);
56785689

56795690
/* get the edge */
56805691
agt_arg = AG_GET_ARG_AGTYPE_P(1);
@@ -5708,8 +5719,6 @@ Datum age_startnode(PG_FUNCTION_ARGS)
57085719

57095720
result = get_vertex(graph_name, label_name, start_id);
57105721

5711-
free(label_name);
5712-
57135722
return result;
57145723
}
57155724

@@ -5738,8 +5747,8 @@ Datum age_endnode(PG_FUNCTION_ARGS)
57385747
Assert(AGT_ROOT_IS_SCALAR(agt_arg));
57395748
agtv_object = get_ith_agtype_value_from_container(&agt_arg->root, 0);
57405749
Assert(agtv_object->type == AGTV_STRING);
5741-
graph_name = strndup(agtv_object->val.string.val,
5742-
agtv_object->val.string.len);
5750+
graph_name = pnstrdup(agtv_object->val.string.val,
5751+
agtv_object->val.string.len);
57435752

57445753
/* get the edge */
57455754
agt_arg = AG_GET_ARG_AGTYPE_P(1);
@@ -5773,8 +5782,6 @@ Datum age_endnode(PG_FUNCTION_ARGS)
57735782

57745783
result = get_vertex(graph_name, label_name, end_id);
57755784

5776-
free(label_name);
5777-
57785785
return result;
57795786
}
57805787

@@ -6401,11 +6408,10 @@ Datum age_tofloat(PG_FUNCTION_ARGS)
64016408
NumericGetDatum(agtv_value->val.numeric)));
64026409
else if (agtv_value->type == AGTV_STRING)
64036410
{
6404-
string = strndup(agtv_value->val.string.val,
6405-
agtv_value->val.string.len);
6411+
string = pnstrdup(agtv_value->val.string.val,
6412+
agtv_value->val.string.len);
64066413
result = float8in_internal_null(string, NULL, "double precision",
64076414
string, &is_valid);
6408-
free(string);
64096415
if (!is_valid)
64106416
PG_RETURN_NULL();
64116417
}
@@ -6703,8 +6709,8 @@ Datum age_tointeger(PG_FUNCTION_ARGS)
67036709
{
67046710
char *endptr;
67056711
/* we need a null terminated cstring */
6706-
string = strndup(agtv_value->val.string.val,
6707-
agtv_value->val.string.len);
6712+
string = pnstrdup(agtv_value->val.string.val,
6713+
agtv_value->val.string.len);
67086714
/* convert it if it is a regular integer string */
67096715
result = strtoi64(string, &endptr, 10);
67106716

@@ -6718,7 +6724,6 @@ Datum age_tointeger(PG_FUNCTION_ARGS)
67186724

67196725
f = float8in_internal_null(string, NULL, "double precision",
67206726
string, &is_valid);
6721-
free(string);
67226727
/*
67236728
* If the conversions failed or it's a special float value,
67246729
* return null.
@@ -6731,10 +6736,6 @@ Datum age_tointeger(PG_FUNCTION_ARGS)
67316736

67326737
result = (int64) f;
67336738
}
6734-
else
6735-
{
6736-
free(string);
6737-
}
67386739
}
67396740
else
67406741
{

‎src/backend/utils/load/ag_load_edges.c‎

Lines changed: 50 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ void edge_field_cb(void *field, size_t field_len, void *data)
3636
if (cr->cur_field == cr->alloc)
3737
{
3838
cr->alloc *= 2;
39-
cr->fields = realloc(cr->fields, sizeof(char *) * cr->alloc);
40-
cr->fields_len = realloc(cr->header, sizeof(size_t *) * cr->alloc);
39+
cr->fields = repalloc_check(cr->fields, sizeof(char *) * cr->alloc);
40+
cr->fields_len = repalloc_check(cr->header, sizeof(size_t *) * cr->alloc);
4141
if (cr->fields == NULL)
4242
{
4343
cr->error = 1;
@@ -48,7 +48,7 @@ void edge_field_cb(void *field, size_t field_len, void *data)
4848
}
4949
cr->fields_len[cr->cur_field] = field_len;
5050
cr->curr_row_length += field_len;
51-
cr->fields[cr->cur_field] = strndup((char*)field, field_len);
51+
cr->fields[cr->cur_field] = pnstrdup((char*)field, field_len);
5252
cr->cur_field += 1;
5353
}
5454

@@ -78,13 +78,13 @@ void edge_row_cb(int delim __attribute__((unused)), void *data)
7878
{
7979
cr->header_num = cr->cur_field;
8080
cr->header_row_length = cr->curr_row_length;
81-
cr->header_len = (size_t* )malloc(sizeof(size_t *) * cr->cur_field);
82-
cr->header = malloc((sizeof (char*) * cr->cur_field));
81+
cr->header_len = (size_t* )palloc(sizeof(size_t *) * cr->cur_field);
82+
cr->header = palloc((sizeof (char*) * cr->cur_field));
8383

8484
for (i = 0; i<cr->cur_field; i++)
8585
{
8686
cr->header_len[i] = cr->fields_len[i];
87-
cr->header[i] = strndup(cr->fields[i], cr->header_len[i]);
87+
cr->header[i] = pnstrdup(cr->fields[i], cr->header_len[i]);
8888
}
8989
}
9090
else
@@ -133,7 +133,7 @@ void edge_row_cb(int delim __attribute__((unused)), void *data)
133133

134134
for (i = 0; i < n_fields; ++i)
135135
{
136-
free(cr->fields[i]);
136+
pfree_if_not_null(cr->fields[i]);
137137
}
138138

139139
if (cr->error)
@@ -192,6 +192,10 @@ int create_edges_from_csv_file(char *file_path,
192192
(errmsg("Failed to initialize csv parser\n")));
193193
}
194194

195+
p.malloc_func = palloc;
196+
p.realloc_func = repalloc_check;
197+
p.free_func = pfree_if_not_null;
198+
195199
csv_set_space_func(&p, is_space);
196200
csv_set_term_func(&p, is_term);
197201

@@ -202,47 +206,52 @@ int create_edges_from_csv_file(char *file_path,
202206
(errmsg("Failed to open %s\n", file_path)));
203207
}
204208

205-
label_seq_name = get_label_seq_relation_name(label_name);
206-
207-
memset((void*)&cr, 0, sizeof(csv_edge_reader));
208-
cr.alloc = 128;
209-
cr.fields = malloc(sizeof(char *) * cr.alloc);
210-
cr.fields_len = malloc(sizeof(size_t *) * cr.alloc);
211-
cr.header_row_length = 0;
212-
cr.curr_row_length = 0;
213-
cr.graph_name = graph_name;
214-
cr.graph_oid = graph_oid;
215-
cr.label_name = label_name;
216-
cr.label_id = label_id;
217-
cr.label_seq_relid = get_relname_relid(label_seq_name, graph_oid);
218-
cr.load_as_agtype = load_as_agtype;
219-
220-
/* Initialize the batch insert state */
221-
init_batch_insert(&cr.batch_state, label_name, graph_oid);
222-
223-
while ((bytes_read=fread(buf, 1, 1024, fp)) > 0)
209+
PG_TRY();
224210
{
225-
if (csv_parse(&p, buf, bytes_read, edge_field_cb,
226-
edge_row_cb, &cr) != bytes_read)
211+
label_seq_name = get_label_seq_relation_name(label_name);
212+
213+
memset((void*)&cr, 0, sizeof(csv_edge_reader));
214+
cr.alloc = 128;
215+
cr.fields = palloc(sizeof(char *) * cr.alloc);
216+
cr.fields_len = palloc(sizeof(size_t *) * cr.alloc);
217+
cr.header_row_length = 0;
218+
cr.curr_row_length = 0;
219+
cr.graph_name = graph_name;
220+
cr.graph_oid = graph_oid;
221+
cr.label_name = label_name;
222+
cr.label_id = label_id;
223+
cr.label_seq_relid = get_relname_relid(label_seq_name, graph_oid);
224+
cr.load_as_agtype = load_as_agtype;
225+
226+
/* Initialize the batch insert state */
227+
init_batch_insert(&cr.batch_state, label_name, graph_oid);
228+
229+
while ((bytes_read=fread(buf, 1, 1024, fp)) > 0)
227230
{
228-
ereport(ERROR, (errmsg("Error while parsing file: %s\n",
229-
csv_strerror(csv_error(&p)))));
231+
if (csv_parse(&p, buf, bytes_read, edge_field_cb,
232+
edge_row_cb, &cr) != bytes_read)
233+
{
234+
ereport(ERROR, (errmsg("Error while parsing file: %s\n",
235+
csv_strerror(csv_error(&p)))));
236+
}
230237
}
231-
}
232238

233-
csv_fini(&p, edge_field_cb, edge_row_cb, &cr);
239+
csv_fini(&p, edge_field_cb, edge_row_cb, &cr);
234240

235-
/* Finish any remaining batch inserts */
236-
finish_batch_insert(&cr.batch_state);
241+
/* Finish any remaining batch inserts */
242+
finish_batch_insert(&cr.batch_state);
237243

238-
if (ferror(fp))
244+
if (ferror(fp))
245+
{
246+
ereport(ERROR, (errmsg("Error while reading file %s\n", file_path)));
247+
}
248+
}
249+
PG_FINALLY();
239250
{
240-
ereport(ERROR, (errmsg("Error while reading file %s\n", file_path)));
251+
fclose(fp);
252+
csv_free(&p);
241253
}
254+
PG_END_TRY();
242255

243-
fclose(fp);
244-
245-
free(cr.fields);
246-
csv_free(&p);
247256
return EXIT_SUCCESS;
248-
}
257+
}

0 commit comments

Comments
 (0)