Skip to content

Conversation

@shihai1991
Copy link
Member

@shihai1991shihai1991 commented Jan 10, 2021

returnNULL;
ModDict=d; /* For PyCurses_InitScr to use later */

void**PyCurses_API=PyMem_Calloc(PyCurses_API_pointers, sizeof(void*));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This API is weird. Would you be interested to add a structure of 4 pointers? Something like PyDateTime_CAPI.

The 4 pointers are used by:

#define PyCursesWindow_Type (*(PyTypeObject *) PyCurses_API[0]) #define PyCursesSetupTermCalled{if (! ((int (*)(void))PyCurses_API[1]) () ) return NULL} #define PyCursesInitialised{if (! ((int (*)(void))PyCurses_API[2]) () ) return NULL} #define PyCursesInitialisedColor{if (! ((int (*)(void))PyCurses_API[3]) () ) return NULL} 

Maybe you can name the 4 members as:

PyTypeObject *Window_Type int SetupTermCalled(void) int Initialised(void) int InitialisedColor(void) 

Or I can merge this PR and then you work on a new PR to add a structure. As you want, tell me what do you prefer ;-)

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or I can merge this PR and then you work on a new PR to add a structure. As you want, tell me what do you prefer ;-)

Oh, I perfer to use another PR to add this structure in this weekend :)

c_api_object=PyCapsule_New(PyCurses_API, PyCurses_CAPSULE_NAME,
curses_destructor);
if (c_api_object==NULL){
PyMem_Free(PyCurses_API);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, there is no a refleak here. Does it work to call curses_destructor() here?

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, thanks, updated.
Weird. ./python Lib/test/regrtest.py -ucurses test_curses -vv -R 3:3 can not catch this refleak.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it work to call curses_destructor() here?

No, it's can not work because capsule object haven't created

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird. ./python Lib/test/regrtest.py -ucurses test_curses -vv -R 3:3 can not catch this refleak.

test_curses doesn't test memory allocation failures. See _testcapi.set_nomemory() for that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, and _curses doesn't use multiphase init API yet.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it.

@vstinnervstinner merged commit 2f12a1b into python:masterJan 22, 2021
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@shihai1991@vstinner@the-knights-who-say-ni@bedevere-bot