Home Original page

bpo-38823: Clean up refleaks in _ast initialization. by brandtbucher · Pull Request #17276 · python/cpython

@brandtbucher

@brandtbucher

@brandtbucher

corona10

Choose a reason for hiding this comment

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

PR looks good to me.
The specific change is that error handling is done by the goto statement
and this decision is looking good to me for this situation.

@vstinner
Please take a look at @brandtbucher 's PR as the core developer.
Thank you for understanding.

vstinner

self.emit("if (!init_types()) return NULL;", 1)
self.emit('m = PyState_FindModule(&_astmodule);', 1)
self.emit("if (!m) return NULL;", 1)
self.emit('if (PyModule_AddObject(m, "AST", astmodulestate_global->AST_type) < 0) goto error;', 1)

Choose a reason for hiding this comment

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

Please add braces, see PEP 7.

self.emit('if (PyModule_AddObject(m, "AST", astmodulestate_global->AST_type) < 0) goto error;', 1)
self.emit('if (PyModule_AddObject(m, "AST", astmodulestate_global->AST_type) < 0) { goto error; }', 1)

Same for changes below.

@csabella

@brandtbucher

@brandtbucher

@brandtbucher

Sorry @vstinner, I didn't realize that PEP 7 applied to generated code. Fixed!

shihai1991

Choose a reason for hiding this comment

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

Thanks, LGTM.

@vstinner

I don't see the need to backport such change. Errors in _ast init function are very unlikely. I prefer to reduce the number of backported changes to stable branches to reduce the risk of regression.

@brandtbucher