[issue43951] Two minor fixes for C module object

[STINNER Victor]

STINNER Victor  added the comment:

commit 175a54b2d8f967605f1d46b5cadccdcf2b54842f
Author: larryhastings 
Date:   Thu Apr 29 20:13:25 2021 -0700

Two minor fixes for accessing a module's name. (#25658)

While working on another issue, I noticed two minor nits in the C 
implementation of the module object.  Both are related to getting a module's 
name.

First, the C function module_dir() (module.__dir__) starts by ensuring the 
module dict is valid.  If the module dict is invalid, it wants to format an 
exception using the name of the module, which it gets from PyModule_GetName().  
However, PyModule_GetName() gets the name of the module from the dict.  So 
getting the name in this circumstance will never succeed.

When module_dir() wants to format the error but can't get the name, it 
knows that PyModule_GetName() must have already raised an exception.  So it 
leaves that exception alone and returns an error.  The end result is that the 
exception raised here is kind of useless and misleading: dir(module) on a 
module with no __dict__ raises SystemError("nameless module").  I changed the 
code to actually raise the exception it wanted to raise, just without a real 
module name: TypeError(".__dict__ is not a dictionary").  This seems 
more useful, and would do a better job putting the programmer who encountered 
this on the right track of figuring out what was going on.

Second, the C API function PyModule_GetNameObject() checks to see if the 
module has a dict.  If m->md_dict is not NULL, it calls 
_PyDict_GetItemIdWithError().  However, it's possible for m->md_dict to be 
None.  And if you call _PyDict_GetItemIdWithError(Py_None, ...) it will *crash*.

Unfortunately, this crash was due to my own bug in the other branch.  
Fixing my code made the crash go away.  I assert that this is still possible at 
the API level.

The fix is easy: add a PyDict_Check() to PyModule_GetNameObject().

Unfortunately, I don't know how to add a unit test for this.  Having 
changed module_dir() above, I can't find any other interfaces callable from 
Python that eventually call PyModule_GetNameObject().  So I don't know how to 
trick the runtime into reproducing this error.

Since both these changes are minor--each entails only a small edit to only 
one line--I didn't bother with a news item.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43951] Two minor fixes for C module object

[Larry Hastings]

Larry Hastings  added the comment:

Thanks for the review, Victor!

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43951] Two minor fixes for C module object

[STINNER Victor]

Change by STINNER Victor :


--
nosy: +vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43951] Two minor fixes for C module object

[Larry Hastings]

Change by Larry Hastings :


--
keywords: +patch
pull_requests: +24349
stage: needs patch -> patch review
pull_request: https://github.com/python/cpython/pull/25658

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43951] Two minor fixes for C module object

[Larry Hastings]

New submission from Larry Hastings :

While working on another issue, I noticed two minor nits in the C 
implementation of the module object.  Both are related to getting a module's 
name.

--

First, the C function module_dir() (module.__dir__) starts by ensuring the 
module dict is valid.  If the module dict is invalid, it wants to format an 
exception using the name of the module, which it gets from PyModule_GetName().  
However, PyModule_GetName() gets the name of the module from the dict.  So 
getting the name in this circumstance will never succeed.

When module_dir() wants to format the error but can't get the name, it knows 
that PyModule_GetName() must have already raised an exception.  So it leaves 
that exception alone and returns an error.  The end result is that the 
exception raised here is kind of useless and misleading: dir(module) on a 
module with no __dict__ raises SystemError("nameless module").  I changed the 
code to actually raise the exception it wanted to raise, just without a real 
module name: TypeError(".__dict__ is not a dictionary").  This seems 
more useful, and would do a better job putting the programmer who encountered 
this on the right track of figuring out what was going on.

--

Second, the C API function PyModule_GetNameObject() checks to see if the module 
has a dict.  If m->md_dict is not NULL, it calls _PyDict_GetItemIdWithError().  
However, it's possible for m->md_dict to be None.  And if you call 
_PyDict_GetItemIdWithError(Py_None, ...) it will *crash*.

Unfortunately, this crash was due to my own bug in the other branch.  Fixing my 
code made the crash go away.  I assert that this is still possible at the API 
level.

The fix is easy: add a PyDict_Check() to PyModule_GetNameObject().

Unfortunately, I don't know how to add a unit test for this.  Having changed 
module_dir() above, I can't find any other interfaces callable from Python that 
eventually call PyModule_GetNameObject().  So I don't know how to trick the 
runtime into reproducing this error.

--
assignee: larry
components: Interpreter Core
messages: 392055
nosy: larry
priority: normal
severity: normal
stage: needs patch
status: open
title: Two minor fixes for C module object
type: behavior
versions: Python 3.10

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com