Issue 18678: Wrong struct members name for spwd module (original) (raw)
Messages (8)
Author: Vajrasky Kok (vajrasky) *
Date: 2013-08-07 14:49
Both python2 and python3 have this behaviour.
import os; os.getuid() 0 'I am root' 'I am root' import spwd spwd.getspnam('bin') spwd.struct_spwd(sp_nam='bin', sp_pwd='*', sp_lstchg=15558, sp_min=0, sp_max=99999, sp_warn=7, sp_inact=-1, sp_expire=-1, sp_flag=-1) spwd.getspnam.doc 'getspnam(name) -> (sp_namp, sp_pwdp, sp_lstchg, sp_min, sp_max,\n sp_warn, sp_inact, sp_expire, sp_flag)\nReturn the shadow password database entry for the given user name.\nSee spwd.doc for more on shadow password database entries.'
The documentation tells the function getspnam will give struct which has member sp_namp and sp_pwdp. But as you can see, the function getspnam gives me a tuple with has member sp_nam (without p) and sp_pwd (without p).
If you "man spwd", you can see the documentation is correct: Structure The shadow password structure is defined in <shadow.h> as follows:
struct spwd {
char *sp_namp; /* Login name */
char *sp_pwdp; /* Encrypted password */
long sp_lstchg; /* Date of last change (measured
in days since 1970-01-01 00:00:00 +0000 (UTC)) */
long sp_min; /* Min # of days between changes */
long sp_max; /* Max # of days between changes */
long sp_warn; /* # of days before password expires
to warn user to change it */
long sp_inact; /* # of days after password expires
until account is disabled */
long sp_expire; /* Date when account expires (measured
in days since 1970-01-01 00:00:00 +0000 (UTC)) */
unsigned long sp_flag; /* Reserved */
};For curious souls who do not have unix box: http://linux.die.net/man/3/getspnam
I have contemplated about whether this behaviour is intended as it is, but I guess this is just a bug. Typo.
Attached the patch to fix this inconsistency. I also fixed some documentation about sp_inact and sp_expire.
I only marked this as Python 3.4 fix because I am not sure whether we should backport it to previous python versions. Some programs that expect sp_nam and sp_pwd names could break.
Author: R. David Murray (r.david.murray) * 
Date: 2013-08-07 14:58
Ideally, for backward compatibility reasons we really ought to support access by the old (incorrect) name even in 3.4 (with a deprecation warning, even more ideally). I'm not sure if that's practical?
Author: Vajrasky Kok (vajrasky) *
Date: 2013-08-18 15:04
Attached the patch to accommodate R. David Murray's request. Accessing invalid attributes such as sp_nam and sp_pwd from spwd tuple will generate deprecation warning message.
Also, I added test function so we can run spwd module directly. This is useful because I don't think we can unit test spwd module. (This module can be used only if you are root.)
The Modules/spwdmodule.c file is supposed to be Modules/_spwdmodule.c following tradition but that will make the review process harder. If this patch is good to go, I'll rename the module file later.
Author: Vajrasky Kok (vajrasky) *
Date: 2013-10-17 04:08
I think giving deprecation message when accessing incorrect attribute from spwd struct is not practical. You're right, R. David Murray, as you can see in spwd_struct_members_name_fix_v2.patch.
So I make a simpler patch. I just give a deprecation message in the source code.
Author: Roundup Robot (python-dev) 
Date: 2013-11-04 00:54
New changeset 1b0ca1a7a3ca by R David Murray in branch 'default': #18678: Correct names of spwd struct members. http://hg.python.org/cpython/rev/1b0ca1a7a3ca
Author: R. David Murray (r.david.murray) *
Date: 2013-11-04 00:57
Thanks, Vajrasky. I elected to apply this only to default, since it hasn't caused any real-world problems. The (small but non-zero) chance of breaking someone's code in the maintenance releases doesn't seem justified by the nature of the fix.
Author: R. David Murray (r.david.murray) *
Date: 2014-03-08 01:23
Revisiting this with fresh eyes, I no longer think this was a typo, and I think we shouldn't have changed it and should change it back (but keep the 'p' names as aliases for those who expect the man page names to be valid).
My logic is: 'sp_namp' is so named because it is a C char pointer to the password. But Python doesn't have pointers in the C sense, so in the python object there is no pointer/non-pointer distinction between sp_namp, sp_pwdp, and all of the rest of the fields. Thus the original decision to drop the 'p', which makes the names easier to type and more consistent with the other field names, there being no pointer distinction in python.
But I do like having the 'p' aliases, as I said above, because someone familiar with the C struct might use them automatically, and since this is a thin wrapper around the C API, we ought to support the C names, I think. And besides, its too late now to remove them ;)
So, I'm reopening this because I'd like to revert the docs, remove the deprecation, and document the 'p' versions as aliases.
Author: Antoine Pitrou (pitrou) *
Date: 2014-03-08 13:51
Agreed with David's latest analysis.
History
Date
User
Action
Args
2022-04-11 14:57:49
admin
set
github: 62878
2014-03-08 13:51:49
pitrou
set
nosy: + pitrou
messages: +
2014-03-08 01:23:40
r.david.murray
set
status: closed -> open
messages: +
stage: resolved -> needs patch
2013-11-04 00:57:14
r.david.murray
set
status: open -> closed
2013-11-04 00:57:02
r.david.murray
set
resolution: fixed
messages: +
stage: resolved
2013-11-04 00:54:27
python-dev
set
nosy: + python-dev
messages: +
2013-10-17 04:08:43
vajrasky
set
files: + spwd_struct_members_name_fix_v3.patch
messages: +
2013-08-18 15:04:48
vajrasky
set
files: + spwd_struct_members_name_fix_v2.patch
messages: +
2013-08-07 14:58:29
r.david.murray
set
nosy: + r.david.murray
messages: +
2013-08-07 14:50:00
vajrasky
create