* Re: [PATCH] linedit, deluser: use POSIX getpwent instead of getpwent_r [not found] ` <CAK1hOcNrEf5MAVqai5P3HjdMMi1pi2LPcABUgDeXWh-ZYscrjg@mail.gmail.com> @ 2015-02-07 16:52 ` Rich Felker 2015-02-09 18:05 ` M Farkas-Dyck 0 siblings, 1 reply; 3+ messages in thread From: Rich Felker @ 2015-02-07 16:52 UTC (permalink / raw) To: Denys Vlasenko; +Cc: busybox, musl On Sat, Feb 07, 2015 at 03:14:10PM +0100, Denys Vlasenko wrote: > On Sat, Feb 7, 2015 at 2:32 AM, Rich Felker <dalias@libc.org> wrote: > >> > 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. > > > > So far we haven't hit anything wanting to use it except busybox. I'm > > all for compatibility, but it doesn't look easy to provide without > > ugly code duplication or similar. We're in the middle of reworking > > some of this code anyway to add alternate backend support, so I just > > asked about how easy it would be to get getpwent_r too, but we didn't > > see any obvious clean ways to do it. This is basically a consequence > > of the way musl uses a dynamic buffer for the strings (line from the > > file) so as not to impose an arbitrary line limit, > > How about this implementation? > > int getpwent_r(struct passwd *pwbuf, char *buf, size_t buflen, struct > passwd **pwbufp) > { > char *line=0; > size_t size=0; > if (!f) f = fopen("/etc/passwd", "rbe"); > if (!f) return 0; > *pwbufp = __getpwent_a(f, pwbuf, &line, &size); > if (!*pwbufp) > return 0; /* success (eof) */ > if (size < buflen) > strcpy(buf, line); > free(line); > if (size < buflen) > return 0; /* success */ > *pwbufp = 0; > errno = ERANGE; > return ERANGE; > } Now *pwbuf contains a bunch of invalid ("dangling") pointers. They need to be fixed-up the same way getpw_r.c is doing it right now, which is where the code duplication comes in. It's not huge but it's ugly having the same logic repeated two places (and again for groups once you do groups too). Rich ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Re: [PATCH] linedit, deluser: use POSIX getpwent instead of getpwent_r 2015-02-07 16:52 ` [PATCH] linedit, deluser: use POSIX getpwent instead of getpwent_r Rich Felker @ 2015-02-09 18:05 ` M Farkas-Dyck 2015-02-09 18:20 ` Rich Felker 0 siblings, 1 reply; 3+ messages in thread From: M Farkas-Dyck @ 2015-02-09 18:05 UTC (permalink / raw) To: musl On 07/02/2015, Rich Felker <dalias@libc.org> wrote: > On Sat, Feb 07, 2015 at 03:14:10PM +0100, Denys Vlasenko wrote: >> On Sat, Feb 7, 2015 at 2:32 AM, Rich Felker <dalias@libc.org> wrote: >> >> > 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. buf may contain arbitrary data, yes? If so we could store the iterator there. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Re: [PATCH] linedit, deluser: use POSIX getpwent instead of getpwent_r 2015-02-09 18:05 ` M Farkas-Dyck @ 2015-02-09 18:20 ` Rich Felker 0 siblings, 0 replies; 3+ messages in thread From: Rich Felker @ 2015-02-09 18:20 UTC (permalink / raw) To: M Farkas-Dyck; +Cc: musl On Mon, Feb 09, 2015 at 01:05:07PM -0500, M Farkas-Dyck wrote: > On 07/02/2015, Rich Felker <dalias@libc.org> wrote: > > On Sat, Feb 07, 2015 at 03:14:10PM +0100, Denys Vlasenko wrote: > >> On Sat, Feb 7, 2015 at 2:32 AM, Rich Felker <dalias@libc.org> wrote: > >> >> > 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. > > buf may contain arbitrary data, yes? If so we could store the iterator there. No. On entry the buffer must be assumed to be uninitialized. The caller could be passing a completely new buffer each time, and even if not, on the first call the buffer is going to contain junk. The contract of getpwent_r, at least on glibc and compatible systems, seems to be that it uses the same iterator as getpwent. But since it turns out there are multiple incompatible historical functions names getpwent_r that all behave differently, this proposal has basically been shelved anyway. Busybox has dropped its use of getpwent_r. Rich ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-02-09 18:20 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <1398411394-19971-1-git-send-email-ncopa@alpinelinux.org> [not found] ` <CAK1hOcMceBFFmQjxV9J6FUBrr_2+uz8y6cT-N6fzhOjWNvxiOA@mail.gmail.com> [not found] ` <20150205184601.GK23507@brightrain.aerifal.cx> [not found] ` <CAK1hOcMBqpz_eT0hU3Rx8aj7rP8wWf5kMpYh-G2qStWEyJi-YQ@mail.gmail.com> [not found] ` <20150205205224.GN23507@brightrain.aerifal.cx> [not found] ` <CAK1hOcPr1ZeaLc8pp=BX=f21xUkX4potRA8UtxDw2khdE+zz_A@mail.gmail.com> [not found] ` <20150207013229.GT23507@brightrain.aerifal.cx> [not found] ` <CAK1hOcNrEf5MAVqai5P3HjdMMi1pi2LPcABUgDeXWh-ZYscrjg@mail.gmail.com> 2015-02-07 16:52 ` [PATCH] linedit, deluser: use POSIX getpwent instead of getpwent_r Rich Felker 2015-02-09 18:05 ` M Farkas-Dyck 2015-02-09 18:20 ` Rich Felker
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/musl/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).