ehlo,
I checked some manual pages, where [ug]?id types are used and each manual page suggest to include header file "sys/types.h". This header file was indirectly included in some files on linux, but it is not portable.
man getgid #include <unistd.h> #include <sys/types.h>
gid_t getgid(void); gid_t getegid(void); ------------------------- man getpwuid #include <sys/types.h> #include <pwd.h>
struct passwd *getpwuid(uid_t uid);
Attached patch modify header file "sss_idmap.h". Should be version-info changed for libsssidmap?
LS
On Mon, Sep 02, 2013 at 07:12:51PM +0200, Lukas Slebodnik wrote:
ehlo,
I checked some manual pages, where [ug]?id types are used and each manual page suggest to include header file "sys/types.h". This header file was indirectly included in some files on linux, but it is not portable.
man getgid #include <unistd.h> #include <sys/types.h>
gid_t getgid(void); gid_t getegid(void);
man getpwuid #include <sys/types.h> #include <pwd.h>
struct passwd *getpwuid(uid_t uid);
I think the change is correct. The POSIX information I checked (http://pubs.opengroup.org/onlinepubs/009695399/basedefs/pwd.h.html for example) say that "The gid_t and uid_t types shall be defined as described in <sys/types.h>."
But I wonder if these are the only files that needed fixing? uid_t seems to be used all over the place..
On (02/09/13 23:02), Jakub Hrozek wrote:
On Mon, Sep 02, 2013 at 07:12:51PM +0200, Lukas Slebodnik wrote:
ehlo,
I checked some manual pages, where [ug]?id types are used and each manual page suggest to include header file "sys/types.h". This header file was indirectly included in some files on linux, but it is not portable.
man getgid #include <unistd.h> #include <sys/types.h>
gid_t getgid(void); gid_t getegid(void);
man getpwuid #include <sys/types.h> #include <pwd.h>
struct passwd *getpwuid(uid_t uid);
I think the change is correct. The POSIX information I checked (http://pubs.opengroup.org/onlinepubs/009695399/basedefs/pwd.h.html for example) say that "The gid_t and uid_t types shall be defined as described in <sys/types.h>."
But I wonder if these are the only files that needed fixing? uid_t seems to be used all over the place..
yes just these two files.
Without this patch, We include "sys/types.h" in 10 header files and 59 "*.c" file.
src/providers/data_provider.h:32:#include <sys/types.h> src/providers/ldap/sdap_async.h:25:#include <sys/types.h> src/providers/proxy/proxy.h:33:#include <sys/types.h> src/responder/sudo/sudosrv_private.h:26:#include <sys/types.h> src/sss_client/sudo/sss_sudo.h:33:#include <sys/types.h> src/util/child_common.h:29:#include <sys/types.h> src/util/find_uid.h:28:#include <sys/types.h> src/util/sss_ldap.h:24:#include <sys/types.h> src/util/sss_nss.h:25:#include <sys/types.h> src/util/util.h:38:#include <sys/types.h> ^^^^^^^ The most important header file :-)
LS
On Mon, Sep 02, 2013 at 07:12:51PM +0200, Lukas Slebodnik wrote:
ehlo,
I checked some manual pages, where [ug]?id types are used and each manual page suggest to include header file "sys/types.h". This header file was indirectly included in some files on linux, but it is not portable.
man getgid #include <unistd.h> #include <sys/types.h>
gid_t getgid(void); gid_t getegid(void);
man getpwuid #include <sys/types.h> #include <pwd.h>
struct passwd *getpwuid(uid_t uid);
Attached patch modify header file "sss_idmap.h". Should be version-info changed for libsssidmap?
LS
good catch, especially for sss_idmap.h. Imo header files should try to be self contained, i.e. does not need extra includes to get loaded properly, and since libsssidmap is a somewhat public it is even more important.
About version-info, if you follow the book (http://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.h...) letter by letter I guess the revison should be incremented but I thin it is not necessary for the given change.
ACK
bye, Sumit
On Tue, Sep 03, 2013 at 10:16:37AM +0200, Sumit Bose wrote:
On Mon, Sep 02, 2013 at 07:12:51PM +0200, Lukas Slebodnik wrote:
ehlo,
I checked some manual pages, where [ug]?id types are used and each manual page suggest to include header file "sys/types.h". This header file was indirectly included in some files on linux, but it is not portable.
man getgid #include <unistd.h> #include <sys/types.h>
gid_t getgid(void); gid_t getegid(void);
man getpwuid #include <sys/types.h> #include <pwd.h>
struct passwd *getpwuid(uid_t uid);
Attached patch modify header file "sss_idmap.h". Should be version-info changed for libsssidmap?
LS
good catch, especially for sss_idmap.h. Imo header files should try to be self contained, i.e. does not need extra includes to get loaded properly, and since libsssidmap is a somewhat public it is even more important.
About version-info, if you follow the book (http://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.h...) letter by letter I guess the revison should be incremented but I thin it is not necessary for the given change.
I think this would only be an issue if someone had their own uid_t implementation somewhere. I think this is OK as-is.
ACK
Pushed to master.
On Tue, Sep 03, 2013 at 02:00:09PM +0200, Jakub Hrozek wrote:
On Tue, Sep 03, 2013 at 10:16:37AM +0200, Sumit Bose wrote:
On Mon, Sep 02, 2013 at 07:12:51PM +0200, Lukas Slebodnik wrote:
ehlo,
I checked some manual pages, where [ug]?id types are used and each manual page suggest to include header file "sys/types.h". This header file was indirectly included in some files on linux, but it is not portable.
man getgid #include <unistd.h> #include <sys/types.h>
gid_t getgid(void); gid_t getegid(void);
man getpwuid #include <sys/types.h> #include <pwd.h>
struct passwd *getpwuid(uid_t uid);
Attached patch modify header file "sss_idmap.h". Should be version-info changed for libsssidmap?
LS
good catch, especially for sss_idmap.h. Imo header files should try to be self contained, i.e. does not need extra includes to get loaded properly, and since libsssidmap is a somewhat public it is even more important.
About version-info, if you follow the book (http://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.h...) letter by letter I guess the revison should be incremented but I thin it is not necessary for the given change.
I think this would only be an issue if someone had their own uid_t implementation somewhere. I think this is OK as-is.
ACK
Pushed to master.
Also pushed to sssd-1-10 and sssd-1-9 (as the BSD will rebase to 1.9)
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Lukas Slebodnik -
Sumit Bose