bpo-1635741: Fix refleaks of timemoudle in PyInit_time()

[shihai1991]

https://bugs.python.org/issue1635741

[shihai1991]

cc @brandtbucher
Due to brandt have doing some refactoring work of PyModule_AddObject() ;)

[brandtbucher]

Thanks for your work on this! Looks good, just one question below:

[pganssle]

Is the reason that you need goto in every one of these blocks that PyErr_Occcurred() will get reset if you make other PyModule_AddIntMacro calls that don't fail along the way?

Either way, I'm mildly tempted to suggest adding a wrapper macro like:

#define _PyModule_AddIntMacroOrGotoError(m, macro) \
    if (PyModule_AddIntMacro(m, macro) < 0) { goto error; }

But I dunno. I'm torn between the principles of "don't repeat yourself" and "don't write a preprocessor macro with a goto in it (you monster)".

[brandtbucher]

Between the two options, I'm personally +0 on repetition. Incorrect error handling for the whole PyModule_Add family of functions is rampant in the stdlib (see my work in bpo-38823), and I think reinforcing the correct usage is good to have to keep people from repeating the same old patterns (especially in outside projects).

[shihai1991]

Is the reason that you need goto in every one of these blocks that PyErr_Occcurred() will get reset if you make other PyModule_AddIntMacro calls that don't fail along the way?

No, I just try to make the Py_DECREF(m);return NULL; looks like a common processing item.

I think reinforcing the correct usage is good to have to keep people from repeating the same old patterns

+1

[vstinner]

I dislike macros hiding a goto. A goto statement is dangerous, I prefer to make it explicit. The "if (...) { goto error; }" is a very common pattern in Python, especially in initialization code.

[vstinner]

Thanks @shihai1991, I merged your PR.