Skip to content

Commit ecf011e

Browse files
authored
Check all allocs in the Arrow tree (#9488)
2 parents cf6de8c + f298638 commit ecf011e

File tree

2 files changed

+46
-33
lines changed

2 files changed

+46
-33
lines changed

src/_imaging.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,9 @@ PyObject *
292292
ExportArrowArrayPyCapsule(ImagingObject *self) {
293293
struct ArrowArray *array =
294294
(struct ArrowArray *)calloc(1, sizeof(struct ArrowArray));
295+
if (!array) {
296+
return ArrowError(IMAGING_CODEC_MEMORY);
297+
}
295298
int err = export_imaging_array(self->image, array);
296299
if (err == 0) {
297300
return PyCapsule_New(array, "arrow_array", ReleaseArrowArrayPyCapsule);

src/libImaging/Arrow.c

Lines changed: 43 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010

1111
static void
1212
ReleaseExportedSchema(struct ArrowSchema *array) {
13-
// This should not be called on already released array
14-
// assert(array->release != NULL);
13+
// TODO here: release and/or deallocate all data directly owned by
14+
// the ArrowArray struct, such as the private_data.
1515

1616
if (!array->release) {
1717
return;
@@ -30,31 +30,36 @@ ReleaseExportedSchema(struct ArrowSchema *array) {
3030
}
3131

3232
// Release children
33-
for (int64_t i = 0; i < array->n_children; ++i) {
34-
struct ArrowSchema *child = array->children[i];
35-
if (child->release != NULL) {
36-
child->release(child);
37-
child->release = NULL;
38-
}
39-
free(array->children[i]);
40-
}
4133
if (array->children) {
34+
for (int64_t i = 0; i < array->n_children; ++i) {
35+
struct ArrowSchema *child = array->children[i];
36+
if (child != NULL) {
37+
if (child->release != NULL) {
38+
child->release(child);
39+
child->release = NULL;
40+
}
41+
free(array->children[i]);
42+
}
43+
}
4244
free(array->children);
45+
array->children = NULL;
4346
}
4447

4548
// Release dictionary
4649
struct ArrowSchema *dict = array->dictionary;
47-
if (dict != NULL && dict->release != NULL) {
48-
dict->release(dict);
49-
dict->release = NULL;
50+
if (dict != NULL) {
51+
if (dict->release != NULL) {
52+
dict->release(dict);
53+
dict->release = NULL;
54+
}
55+
free(dict);
56+
array->dictionary = NULL;
5057
}
5158

52-
// TODO here: release and/or deallocate all data directly owned by
53-
// the ArrowArray struct, such as the private_data.
54-
5559
// Mark array released
5660
array->release = NULL;
5761
}
62+
5863
char *
5964
image_band_json(Imaging im) {
6065
char *format = "{\"bands\": [\"%s\", \"%s\", \"%s\", \"%s\"]}";
@@ -220,13 +225,19 @@ export_imaging_schema(Imaging im, struct ArrowSchema *schema) {
220225
// if it's not 1 band, it's an int32 at the moment. 4 uint8 bands.
221226
schema->n_children = 1;
222227
schema->children = calloc(1, sizeof(struct ArrowSchema *));
228+
if (!schema->children) {
229+
schema->release(schema);
230+
return IMAGING_CODEC_MEMORY;
231+
}
223232
schema->children[0] = (struct ArrowSchema *)calloc(1, sizeof(struct ArrowSchema));
233+
if (!schema->children[0]) {
234+
schema->release(schema);
235+
return IMAGING_CODEC_MEMORY;
236+
}
224237
retval = export_named_type(
225238
schema->children[0], im->arrow_band_format, getModeData(im->mode)->name
226239
);
227240
if (retval != 0) {
228-
free(schema->children[0]);
229-
free(schema->children);
230241
schema->release(schema);
231242
return retval;
232243
}
@@ -256,11 +267,12 @@ release_const_array(struct ArrowArray *array) {
256267
array->buffers = NULL;
257268
}
258269
if (array->children) {
259-
// undone -- does arrow release all the children recursively?
260270
for (int i = 0; i < array->n_children; i++) {
261-
if (array->children[i]->release) {
262-
array->children[i]->release(array->children[i]);
263-
array->children[i]->release = NULL;
271+
if (array->children[i]) {
272+
if (array->children[i]->release) {
273+
array->children[i]->release(array->children[i]);
274+
array->children[i]->release = NULL;
275+
}
264276
free(array->children[i]);
265277
}
266278
}
@@ -303,8 +315,11 @@ export_single_channel_array(Imaging im, struct ArrowArray *array) {
303315
};
304316

305317
// Allocate list of buffers
306-
array->buffers = (const void **)malloc(sizeof(void *) * array->n_buffers);
307-
// assert(array->buffers != NULL);
318+
array->buffers = (const void **)calloc(1, sizeof(void *) * array->n_buffers);
319+
if (!array->buffers) {
320+
array->release(array);
321+
return IMAGING_CODEC_MEMORY;
322+
}
308323
array->buffers[0] = NULL; // no nulls, null bitmap can be omitted
309324

310325
if (im->block) {
@@ -386,6 +401,9 @@ export_fixed_pixel_array(Imaging im, struct ArrowArray *array) {
386401

387402
array->children[0]->buffers =
388403
(const void **)calloc(2, sizeof(void *) * array->n_buffers);
404+
if (!array->children[0]->buffers) {
405+
goto err;
406+
}
389407

390408
if (im->block) {
391409
array->children[0]->buffers[1] = im->block;
@@ -395,15 +413,7 @@ export_fixed_pixel_array(Imaging im, struct ArrowArray *array) {
395413
return 0;
396414

397415
err:
398-
if (array->children[0]) {
399-
free(array->children[0]);
400-
}
401-
if (array->children) {
402-
free(array->children);
403-
}
404-
if (array->buffers) {
405-
free(array->buffers);
406-
}
416+
array->release(array);
407417
return IMAGING_CODEC_MEMORY;
408418
}
409419

0 commit comments

Comments
 (0)