Skip to content

Commit 816bd77

Browse files
authored
Batch and error check allocations (#3224)
* Batch PyMem_New-s in draw routines * Batch and error check mallocs in blur routines * Move PyErr_NoMemory out of GIL-less block * Remove misplaced PyMem_Free
1 parent 28db8df commit 816bd77

File tree

2 files changed

+53
-48
lines changed

2 files changed

+53
-48
lines changed

src_c/draw.c

Lines changed: 16 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,6 @@ aalines(PyObject *self, PyObject *arg, PyObject *kwargs)
279279
Uint32 color;
280280
float pts[4];
281281
float pts_prev[4];
282-
float *xlist, *ylist;
283282
float x, y;
284283
int l, t;
285284
int extra_px;
@@ -333,16 +332,12 @@ aalines(PyObject *self, PyObject *arg, PyObject *kwargs)
333332
"points argument must contain 2 or more points");
334333
}
335334

336-
xlist = PyMem_New(float, length);
337-
ylist = PyMem_New(float, length);
335+
// Allocate bytes for the xlist and ylist at once to reduce allocations.
336+
float *points_buf = PyMem_New(float, length * 2);
337+
float *xlist = points_buf;
338+
float *ylist = points_buf + length;
338339

339-
if (NULL == xlist || NULL == ylist) {
340-
if (xlist) {
341-
PyMem_Free(xlist);
342-
}
343-
if (ylist) {
344-
PyMem_Free(ylist);
345-
}
340+
if (points_buf == NULL) {
346341
return RAISE(PyExc_MemoryError,
347342
"cannot allocate memory to draw aalines");
348343
}
@@ -357,8 +352,7 @@ aalines(PyObject *self, PyObject *arg, PyObject *kwargs)
357352
Py_DECREF(item);
358353

359354
if (!result) {
360-
PyMem_Free(xlist);
361-
PyMem_Free(ylist);
355+
PyMem_Free(points_buf);
362356
return RAISE(PyExc_TypeError, "points must be number pairs");
363357
}
364358

@@ -367,8 +361,7 @@ aalines(PyObject *self, PyObject *arg, PyObject *kwargs)
367361
}
368362

369363
if (!pgSurface_Lock(surfobj)) {
370-
PyMem_Free(xlist);
371-
PyMem_Free(ylist);
364+
PyMem_Free(points_buf);
372365
return RAISE(PyExc_RuntimeError, "error locking surface");
373366
}
374367

@@ -460,8 +453,7 @@ aalines(PyObject *self, PyObject *arg, PyObject *kwargs)
460453
disable_endpoints, disable_endpoints, extra_px);
461454
}
462455

463-
PyMem_Free(xlist);
464-
PyMem_Free(ylist);
456+
PyMem_Free(points_buf);
465457

466458
if (!pgSurface_Unlock(surfobj)) {
467459
return RAISE(PyExc_RuntimeError, "error unlocking surface");
@@ -970,7 +962,6 @@ polygon(PyObject *self, PyObject *arg, PyObject *kwargs)
970962
PyObject *colorobj, *points, *item = NULL;
971963
SDL_Surface *surf = NULL;
972964
Uint32 color;
973-
int *xlist = NULL, *ylist = NULL;
974965
int width = 0; /* Default width. */
975966
int x, y, result, l, t;
976967
int drawn_area[4] = {INT_MAX, INT_MAX, INT_MIN,
@@ -1021,16 +1012,12 @@ polygon(PyObject *self, PyObject *arg, PyObject *kwargs)
10211012
"points argument must contain more than 2 points");
10221013
}
10231014

1024-
xlist = PyMem_New(int, length);
1025-
ylist = PyMem_New(int, length);
1015+
// Allocate bytes for the xlist and ylist at once to reduce allocations.
1016+
int *points_buf = PyMem_New(int, length * 2);
1017+
int *xlist = points_buf;
1018+
int *ylist = points_buf + length;
10261019

1027-
if (NULL == xlist || NULL == ylist) {
1028-
if (xlist) {
1029-
PyMem_Free(xlist);
1030-
}
1031-
if (ylist) {
1032-
PyMem_Free(ylist);
1033-
}
1020+
if (points_buf == NULL) {
10341021
return RAISE(PyExc_MemoryError,
10351022
"cannot allocate memory to draw polygon");
10361023
}
@@ -1045,8 +1032,7 @@ polygon(PyObject *self, PyObject *arg, PyObject *kwargs)
10451032
Py_DECREF(item);
10461033

10471034
if (!result) {
1048-
PyMem_Free(xlist);
1049-
PyMem_Free(ylist);
1035+
PyMem_Free(points_buf);
10501036
return RAISE(PyExc_TypeError, "points must be number pairs");
10511037
}
10521038

@@ -1055,8 +1041,7 @@ polygon(PyObject *self, PyObject *arg, PyObject *kwargs)
10551041
}
10561042

10571043
if (!pgSurface_Lock(surfobj)) {
1058-
PyMem_Free(xlist);
1059-
PyMem_Free(ylist);
1044+
PyMem_Free(points_buf);
10601045
return RAISE(PyExc_RuntimeError, "error locking surface");
10611046
}
10621047

@@ -1066,8 +1051,7 @@ polygon(PyObject *self, PyObject *arg, PyObject *kwargs)
10661051
else {
10671052
draw_filltri(surf, xlist, ylist, color, drawn_area);
10681053
}
1069-
PyMem_Free(xlist);
1070-
PyMem_Free(ylist);
1054+
PyMem_Free(points_buf);
10711055

10721056
if (!pgSurface_Unlock(surfobj)) {
10731057
return RAISE(PyExc_RuntimeError, "error unlocking surface");

src_c/transform.c

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3562,7 +3562,7 @@ surf_average_color(PyObject *self, PyObject *args, PyObject *kwargs)
35623562
return Py_BuildValue("(bbbb)", r, g, b, a);
35633563
}
35643564

3565-
static void
3565+
static int
35663566
box_blur(SDL_Surface *src, SDL_Surface *dst, int radius, SDL_bool repeat)
35673567
{
35683568
// Reference : https://blog.csdn.net/blogshinelee/article/details/80997324
@@ -3574,9 +3574,16 @@ box_blur(SDL_Surface *src, SDL_Surface *dst, int radius, SDL_bool repeat)
35743574
int dst_pitch = dst->pitch;
35753575
int src_pitch = src->pitch;
35763576
int i, x, y, color;
3577-
Uint32 *buf = malloc(dst_pitch * sizeof(Uint32));
3578-
Uint32 *sum_v = malloc(dst_pitch * sizeof(Uint32));
3579-
Uint32 *sum_h = malloc(nb * sizeof(Uint32));
3577+
3578+
// Allocate bytes for buf, sum_v, and sum_h at once to reduce allocations.
3579+
Uint32 *overall_buf = malloc(sizeof(Uint32) * (dst_pitch * 2 + nb));
3580+
Uint32 *buf = overall_buf;
3581+
Uint32 *sum_v = overall_buf + dst_pitch;
3582+
Uint32 *sum_h = overall_buf + dst_pitch * 2;
3583+
3584+
if (overall_buf == NULL) {
3585+
return -1;
3586+
}
35803587

35813588
memset(sum_v, 0, dst_pitch * sizeof(Uint32));
35823589
for (y = 0; y <= radius; y++) { // y-pre
@@ -3641,12 +3648,11 @@ box_blur(SDL_Surface *src, SDL_Surface *dst, int radius, SDL_bool repeat)
36413648
}
36423649
}
36433650

3644-
free(buf);
3645-
free(sum_v);
3646-
free(sum_h);
3651+
free(overall_buf);
3652+
return 0;
36473653
}
36483654

3649-
static void
3655+
static int
36503656
gaussian_blur(SDL_Surface *src, SDL_Surface *dst, int sigma, SDL_bool repeat)
36513657
{
36523658
Uint8 *srcpx = (Uint8 *)src->pixels;
@@ -3657,11 +3663,19 @@ gaussian_blur(SDL_Surface *src, SDL_Surface *dst, int sigma, SDL_bool repeat)
36573663
int src_pitch = src->pitch;
36583664
int i, j, x, y, color;
36593665
int kernel_radius = sigma * 2;
3660-
float *buf = malloc(dst_pitch * sizeof(float));
3661-
float *buf2 = malloc(dst_pitch * sizeof(float));
3662-
float *lut = malloc((kernel_radius + 1) * sizeof(float));
36633666
float lut_sum = 0.0;
36643667

3668+
// Allocate bytes for buf, buf2, and lut at once to reduce allocations.
3669+
float *overall_buf =
3670+
malloc(sizeof(float) * (dst_pitch * 2 + kernel_radius + 1));
3671+
float *buf = overall_buf;
3672+
float *buf2 = overall_buf + dst_pitch;
3673+
float *lut = overall_buf + dst_pitch * 2;
3674+
3675+
if (overall_buf == NULL) {
3676+
return -1;
3677+
}
3678+
36653679
for (i = 0; i <= kernel_radius; i++) { // init gaussian lut
36663680
// Gaussian function
36673681
lut[i] =
@@ -3723,9 +3737,8 @@ gaussian_blur(SDL_Surface *src, SDL_Surface *dst, int sigma, SDL_bool repeat)
37233737
}
37243738
}
37253739

3726-
free(buf);
3727-
free(buf2);
3728-
free(lut);
3740+
free(overall_buf);
3741+
return 0;
37293742
}
37303743

37313744
static SDL_Surface *
@@ -3734,6 +3747,7 @@ blur(pgSurfaceObject *srcobj, pgSurfaceObject *dstobj, int radius,
37343747
{
37353748
SDL_Surface *src = NULL;
37363749
SDL_Surface *retsurf = NULL;
3750+
int result = 0;
37373751

37383752
if (radius < 0) {
37393753
return RAISE(PyExc_ValueError,
@@ -3793,17 +3807,24 @@ blur(pgSurfaceObject *srcobj, pgSurfaceObject *dstobj, int radius,
37933807
Py_BEGIN_ALLOW_THREADS;
37943808

37953809
if (algorithm == 'b') {
3796-
box_blur(src, retsurf, radius, repeat);
3810+
result = box_blur(src, retsurf, radius, repeat);
37973811
}
37983812
else if (algorithm == 'g') {
3799-
gaussian_blur(src, retsurf, radius, repeat);
3813+
result = gaussian_blur(src, retsurf, radius, repeat);
38003814
}
38013815

38023816
Py_END_ALLOW_THREADS;
38033817

38043818
pgSurface_Unlock(srcobj);
38053819
SDL_UnlockSurface(retsurf);
38063820

3821+
// Routines only set error flag if memory allocation failed
3822+
// Setting Python exception here outside of Py_ THREADS block to be safe
3823+
if (result) {
3824+
PyErr_NoMemory();
3825+
return NULL;
3826+
}
3827+
38073828
return retsurf;
38083829
}
38093830

0 commit comments

Comments
 (0)