classification
Title: Wrong struct members name for spwd module
Type: behavior Stage: needs patch
Components: Extension Modules Versions: Python 3.4
process
Status: open Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: pitrou, python-dev, r.david.murray, vajrasky
Priority: normal Keywords: patch

Created on 2013-08-07 14:50 by vajrasky, last changed 2014-03-08 13:51 by pitrou.

Files
File name Uploaded Description Edit
spwd_struct_members_name_fix.patch vajrasky, 2013-08-07 14:49 review
spwd_struct_members_name_fix_v2.patch vajrasky, 2013-08-18 15:04 review
spwd_struct_members_name_fix_v3.patch vajrasky, 2013-10-17 04:08 review
Messages (8)
msg194618 - (view) 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.
msg194619 - (view) Author: R. David Murray (r.david.murray) * (Python committer) 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?
msg195566 - (view) 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.
msg200107 - (view) 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.
msg202074 - (view) 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
msg202075 - (view) Author: R. David Murray (r.david.murray) * (Python committer) 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.
msg212917 - (view) Author: R. David Murray (r.david.murray) * (Python committer) 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.
msg212930 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-03-08 13:51
Agreed with David's latest analysis.
History
Date User Action Args
2014-03-08 13:51:49pitrousetnosy: + pitrou
messages: + msg212930
2014-03-08 01:23:40r.david.murraysetstatus: closed -> open

messages: + msg212917
stage: resolved -> needs patch
2013-11-04 00:57:14r.david.murraysetstatus: open -> closed
2013-11-04 00:57:02r.david.murraysetresolution: fixed
messages: + msg202075
stage: resolved
2013-11-04 00:54:27python-devsetnosy: + python-dev
messages: + msg202074
2013-10-17 04:08:43vajraskysetfiles: + spwd_struct_members_name_fix_v3.patch

messages: + msg200107
2013-08-18 15:04:48vajraskysetfiles: + spwd_struct_members_name_fix_v2.patch

messages: + msg195566
2013-08-07 14:58:29r.david.murraysetnosy: + r.david.murray
messages: + msg194619
2013-08-07 14:50:00vajraskycreate