- Notifications
You must be signed in to change notification settings - Fork3.8k
fixed segfault#3666
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
fixed segfault#3666
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm fairly certain it would be important to add this check to other places where it hasn't been added:
Lines 432 to 438 in25aac95
| staticPyObject* | |
| _pxarray_get_itemsize(pgPixelArrayObject*self,void*closure) | |
| { | |
| SDL_Surface*surf=pgSurface_AsSurface(self->surface); | |
| returnPyLong_FromLong((long)surf->format->BytesPerPixel); | |
| } |
and I'm not sure about this but ifshape also gets set to NULL (even if that doesn't happen, I suppose these should then still check ifself->surface == NULL):
Lines 443 to 450 in25aac95
| staticPyObject* | |
| _pxarray_get_shape(pgPixelArrayObject*self,void*closure) | |
| { | |
| if (self->shape[1]) { | |
| returnPy_BuildValue("(nn)",self->shape[0],self->shape[1]); | |
| } | |
| returnPy_BuildValue("(n)",self->shape[0]); | |
| } |
Lines 455 to 462 in25aac95
| staticPyObject* | |
| _pxarray_get_strides(pgPixelArrayObject*self,void*closure) | |
| { | |
| if (self->shape[1]) { | |
| returnPy_BuildValue("(nn)",self->strides[0],self->strides[1]); | |
| } | |
| returnPy_BuildValue("(n)",self->strides[0]); | |
| } |
Lines 467 to 471 in25aac95
| staticPyObject* | |
| _pxarray_get_ndim(pgPixelArrayObject*self,void*closure) | |
| { | |
| returnPyLong_FromLong(self->shape[1] ?2L :1L); | |
| } |
And I'm fairly certain there are a couple more functions that need this check, perhaps, also inpixelarray_methods.c
MyreMylar commentedJan 14, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
From my testing these are the PixelArray public functions and members that segfault after a first call to
The others already say: Or return some valid data. I didn't bother testing all the dunder methods because this is already a fairly dumb thing for a user to be doing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Please add a similar check to clear the other segfaults outlined above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM 👍
This looks reasonable to me, thanks Andrew! |
Fixes#3665 by raising a
ValueErrorif the surface access is after closing thePixelArray