Re: [PATCH] linedit, deluser: use POSIX getpwent instead of getpwent_r
On Sat, Feb 7, 2015 at 3:48 PM, tito farmat...@tiscali.it wrote: char *line=0; size_t size=0; if (!f) f = fopen(/etc/passwd, rbe); if (!f) return 0; Hi, this should return ENOENT You are correct. *pwbufp = __getpwent_a(f, pwbuf, line, size); if (!*pwbufp) return 0; /* success (eof) */ this should return ENOENT on EOF and 0 otherwise or you will be stuck in infite loops. This is definitely EOF, not a success, we got next entry (since pw == NULL), so should be return ENOENT. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] linedit, deluser: use POSIX getpwent instead of getpwent_r
On Sat, Feb 07, 2015 at 09:49:19AM -0800, Isaac Dunham wrote:
On Thu, Feb 05, 2015 at 03:52:24PM -0500, Rich Felker wrote:
On Thu, Feb 05, 2015 at 09:42:08PM +0100, Denys Vlasenko wrote:
struct passwd *getpwent()
{
static char *line;
static struct passwd pw;
size_t size=0;
if (!f) f = fopen(/etc/passwd, rbe);
if (!f) return 0;
return __getpwent_a(f, pw, line, size);
}
I would prefer that even struct passwd is malloced...
I don't think it would make much practical difference. It could be
changed though.
But more importantly, bbox can't optimize only for musl.
Other libc'es may have static line buffers there.
And musl will eventually be forced to implement getpwent_r()
if it wants to be usable for more packages... so...
getpwent_r makes no sense; the _r functions are for thread-safe
versions of their corresponding legacy functions, but getpwent_r has
inherent global state -- the iterator. Whoever made it just wasn't
thinking. To make a correct interface like this the caller would need
to have an iterator object to pass to the function, but I can't see
much merit in inventing a new interface for this.
Besides having hidden global state, the man page notes:
Other systems use the prototype
struct passwd *getpwent_r(struct passwd *pwd, char *buf, int buflen);
or, better,
int getpwent_r(struct passwd *pwd, char *buf, int buflen, FILE **pw_fp);
In other words, according to the manpage, getpwent_r() is decidedly
unportable.
Per my investigations, Dragonfly/Net/FreeBSD seem to use the same
prototype as glibc; apparently Solaris uses the first alternate prototype;
and the last mentioned seems to be a reference to Tru64, which
uses pw_fp to keep track of its position instead of an iterator.
OpenBSD and MirBSD do not implement getpwent_r, as far as I can tell.
It should be noted here that multiple conflicting historical
definitions of a nonstandard interface are one of the big exclusion
criteria musl goes by.
Rich
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] linedit, deluser: use POSIX getpwent instead of getpwent_r
On Thu, Feb 05, 2015 at 03:52:24PM -0500, Rich Felker wrote:
On Thu, Feb 05, 2015 at 09:42:08PM +0100, Denys Vlasenko wrote:
struct passwd *getpwent()
{
static char *line;
static struct passwd pw;
size_t size=0;
if (!f) f = fopen(/etc/passwd, rbe);
if (!f) return 0;
return __getpwent_a(f, pw, line, size);
}
I would prefer that even struct passwd is malloced...
I don't think it would make much practical difference. It could be
changed though.
But more importantly, bbox can't optimize only for musl.
Other libc'es may have static line buffers there.
And musl will eventually be forced to implement getpwent_r()
if it wants to be usable for more packages... so...
getpwent_r makes no sense; the _r functions are for thread-safe
versions of their corresponding legacy functions, but getpwent_r has
inherent global state -- the iterator. Whoever made it just wasn't
thinking. To make a correct interface like this the caller would need
to have an iterator object to pass to the function, but I can't see
much merit in inventing a new interface for this.
Besides having hidden global state, the man page notes:
Other systems use the prototype
struct passwd *getpwent_r(struct passwd *pwd, char *buf, int buflen);
or, better,
int getpwent_r(struct passwd *pwd, char *buf, int buflen, FILE **pw_fp);
In other words, according to the manpage, getpwent_r() is decidedly
unportable.
Per my investigations, Dragonfly/Net/FreeBSD seem to use the same
prototype as glibc; apparently Solaris uses the first alternate prototype;
and the last mentioned seems to be a reference to Tru64, which
uses pw_fp to keep track of its position instead of an iterator.
OpenBSD and MirBSD do not implement getpwent_r, as far as I can tell.
HTH,
Isaac Dunham
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] linedit, deluser: use POSIX getpwent instead of getpwent_r
On Fri, Apr 25, 2014 at 9:36 AM, Natanael Copa nc...@alpinelinux.org wrote: Prefer use POSIX getpwent over glibc extension getpwent_r. This fixes building with musl libc with CONFIG_USE_BB_PWD_GRP disabled. Sorry, I missed this patch. How much static data (data or bss increase according to size busybox) do these functions pull in in static build? The comments specifically say they use getpwent_r because that avoids static data size penalty, usually about 1k. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] linedit, deluser: use POSIX getpwent instead of getpwent_r
On Thu, Feb 05, 2015 at 09:42:08PM +0100, Denys Vlasenko wrote:
struct passwd *getpwent()
{
static char *line;
static struct passwd pw;
size_t size=0;
if (!f) f = fopen(/etc/passwd, rbe);
if (!f) return 0;
return __getpwent_a(f, pw, line, size);
}
I would prefer that even struct passwd is malloced...
I don't think it would make much practical difference. It could be
changed though.
But more importantly, bbox can't optimize only for musl.
Other libc'es may have static line buffers there.
And musl will eventually be forced to implement getpwent_r()
if it wants to be usable for more packages... so...
getpwent_r makes no sense; the _r functions are for thread-safe
versions of their corresponding legacy functions, but getpwent_r has
inherent global state -- the iterator. Whoever made it just wasn't
thinking. To make a correct interface like this the caller would need
to have an iterator object to pass to the function, but I can't see
much merit in inventing a new interface for this.
Rich
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] linedit, deluser: use POSIX getpwent instead of getpwent_r
On Thu, Feb 5, 2015 at 9:52 PM, Rich Felker dal...@libc.org wrote: And musl will eventually be forced to implement getpwent_r() if it wants to be usable for more packages... so... getpwent_r makes no sense; The _r part does not make sense; the fact that it avoids static storage _is_ useful. the _r functions are for thread-safe versions of their corresponding legacy functions, but getpwent_r has inherent global state -- the iterator. Whoever made it just wasn't thinking. To make a correct interface like this the caller would need to have an iterator object to pass to the function, but I can't see much merit in inventing a new interface for this. It doesn't matter that it makes no sense. It's glibc compat. If you want more programs rather than less to build against musl, you have to strive to be glibc-compatible where practical. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] linedit, deluser: use POSIX getpwent instead of getpwent_r
On Thu, Feb 05, 2015 at 02:09:36PM +0100, Denys Vlasenko wrote: On Fri, Apr 25, 2014 at 9:36 AM, Natanael Copa nc...@alpinelinux.org wrote: Prefer use POSIX getpwent over glibc extension getpwent_r. This fixes building with musl libc with CONFIG_USE_BB_PWD_GRP disabled. Sorry, I missed this patch. How much static data (data or bss increase according to size busybox) do these functions pull in in static build? The comments specifically say they use getpwent_r because that avoids static data size penalty, usually about 1k. The static size penalty on musl is a few bytes. We don't have full static buffers for the strings but just use the buffer getline() produces while reading the file, so only this pointer and the passwd struct itself, not the strings it points to, have static storage. Rich ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] linedit, deluser: use POSIX getpwent instead of getpwent_r
struct passwd *getpwent()
{
static char *line;
static struct passwd pw;
size_t size=0;
if (!f) f = fopen(/etc/passwd, rbe);
if (!f) return 0;
return __getpwent_a(f, pw, line, size);
}
I would prefer that even struct passwd is malloced...
But more importantly, bbox can't optimize only for musl.
Other libc'es may have static line buffers there.
And musl will eventually be forced to implement getpwent_r()
if it wants to be usable for more packages... so...
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] linedit, deluser: use POSIX getpwent instead of getpwent_r
Prefer use POSIX getpwent over glibc extension getpwent_r. This fixes
building with musl libc with CONFIG_USE_BB_PWD_GRP disabled.
Signed-off-by: Natanael Copa nc...@alpinelinux.org
---
libbb/lineedit.c | 11 ---
loginutils/deluser.c | 11 +--
2 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/libbb/lineedit.c b/libbb/lineedit.c
index 8564307..99e6e2c 100644
--- a/libbb/lineedit.c
+++ b/libbb/lineedit.c
@@ -672,20 +672,17 @@ static char *username_path_completion(char *ud)
*/
static NOINLINE unsigned complete_username(const char *ud)
{
- /* Using _r function to avoid pulling in static buffers */
- char line_buff[256];
- struct passwd pwd;
- struct passwd *result;
+ struct passwd *pw;
unsigned userlen;
ud++; /* skip ~ */
userlen = strlen(ud);
setpwent();
- while (!getpwent_r(pwd, line_buff, sizeof(line_buff), result)) {
+ while ((pw = getpwent())) {
/* Null usernames should result in all users as possible
completions. */
- if (/*!userlen || */ strncmp(ud, pwd.pw_name, userlen) == 0) {
- add_match(xasprintf(~%s/, pwd.pw_name));
+ if (/*!userlen || */ strncmp(ud, pw-pw_name, userlen) == 0) {
+ add_match(xasprintf(~%s/, pw-pw_name));
}
}
endpwent();
diff --git a/loginutils/deluser.c b/loginutils/deluser.c
index e39ac55..d7d9b24 100644
--- a/loginutils/deluser.c
+++ b/loginutils/deluser.c
@@ -73,14 +73,13 @@ int deluser_main(int argc, char **argv)
if (!member) {
/* delgroup GROUP */
struct passwd *pw;
- struct passwd pwent;
/* Check if the group is in use */
-#define passwd_buf bb_common_bufsiz1
- while (!getpwent_r(pwent, passwd_buf,
sizeof(passwd_buf), pw)) {
- if (pwent.pw_gid == gr-gr_gid)
- bb_error_msg_and_die('%s'
still has '%s' as their primary group!, pwent.pw_name, name);
+ setpwent();
+ while ((pw = getpwent())) {
+ if (pw-pw_gid == gr-gr_gid)
+ bb_error_msg_and_die('%s'
still has '%s' as their primary group!, pw-pw_name, name);
}
- //endpwent();
+ endpwent();
}
pfile = bb_path_group_file;
if (ENABLE_FEATURE_SHADOWPASSWDS)
--
1.9.2
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
