homepage

As root:
>>> import spwd
>>> spwd.getspall()
[spwd.struct_spwd(sp_nam='root', sp_pwd='!', sp_lstchg=14895, sp_min=0, sp_max=99999, sp_warn=7, sp_inact=-1, sp_expire=-1, sp_flag=-1)
 ...
]
As limited user:
>>> import spwd
>>> spwd.getspall()
[]
>>> 
Wouldn't it be better for consistency to raise OSError EACCES instead?

See also issue18787.

Attached the patch to raise OSError instead of returning empty list if a normal user uses getspall function of spwd module.
I investigated about this issue. There exists situation where /etc/shadow file does not exist:
https://bugs.kde.org/show_bug.cgi?id=117202
https://dev.openwrt.org/ticket/2887
So I think it is better to differentiate the result of the function where a normal user trying to access /etc/shadow file (permission denied, therefore should raise exception) or root user trying to access non-existent /etc/shadow file (therefore should get empty list).

Looking back at this: considering we may get errors != EACCESS I think we better be as generic as possible as in:
--- a/Modules/spwdmodule.c
+++ b/Modules/spwdmodule.c
@@ -153,6 +153,8 @@
 if ((d = PyList_New(0)) == NULL)
 return NULL;
 setspent();
+ if (errno != 0)
+ return PyErr_SetFromErrno(PyExc_OSError);
 while ((p = getspent()) != NULL) {
 PyObject *v = mkspent(p);
 if (v == NULL || PyList_Append(d, v) != 0) {
As for 2.7 and 3.3 I have a little concern in terms of backward compatibility as the user will suddenly receive an exception instead of [] but I think that for the sake of correctness the cost is justified.

Here is the updated patch addressing Giampaolo's concern.

LGTM

I doubt that we can change behavior at this time.

You mean this should be made in 3.4 only?

Even for 3.4 it looks too late.
But I afraid we can't change this behavior never. Users of spwd don't expect 
any exception, raising an exception will break any existing code.
Only one safe way is design new API and deprecate old API. This will be great 
refactoring, it should also change APIs of the pwd and grp modules, perhaps 
these three modules should be merged, perhaps we should cross-platform API 
(available on Windows).

I can agree with you that it's probably better to avoid changing existent python versions to avoid breakage but I see no need to be that strict for newer ones.
Current version should check errno but it doesn't. IMO that is clearly a bug.
Same goes for returning an empty list when users actually exists and for *not* raising FileNotFoundError if the shadow file is not available.
Also note that the doc does not mention that an empty list should be treated as an alias for "insufficient permissions" or "shadow file not available", therefore whoever made that assumption is someone who deliberately and erroneously decided to do so.

I think that a new cross platform API would be great, but I don't think that fixing this bug should wait for that. It may be borderline for being changed in a feature release (and definitely should not be changed in a maintenance release), but I think it would be OK. Mostly likely a program that uses spwd will also be using other root-only functions and already be producing other errors if run as a non-privileged user. The file-doesn't-exist case is a little more concerning, but my guess is that anyone actually checking for an empty return (as opposed to their program just failing when it gets one) would actually welcome the change, since it will give a more specific error message.
Or, to put it another way, I think the negative impact of this in a feature release will be low enough that we won't take much if any flak for it :)

I still think this should be fixed (raise an exception) in the next major release.

Unless Serhiy (or someone else) still objects, I say go ahead and commit it to default.

Adding a test would be a good idea, though...

This issue should be considered in conjunction with issue18787. And if we decise to break backward compatibility, this should be well documented in What's New.
May be discuss these issues on Python-Dev?

The approach in the patch seems good to me.

Too late for 3.5 for this IMO.

The man page for setspent says this:
"The functions that return a pointer return NULL if no more entries are available or if an error occurs during processing. The functions which
 have int as the return value return 0 for success and -1 for failure, with errno set to indicate the cause of the error.
 For the nonreentrant functions, the return value may point to static area, and may be overwritten by subsequent calls to these functions.
 The reentrant functions return zero on success. In case of error, an error number is returned.
"
-> that is, setspent's interaction with errno is undefined, at least on Linux. I'm a little worried about whether this may pickup false errors as a result.

I'm moving this back to patch review - it needs a test, particularly because of the question I have around setspent.