Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Sat, 7 Feb 2015 11:52:19 -0500
From: Rich Felker <dalias@...c.org>
To: Denys Vlasenko <vda.linux@...glemail.com>
Cc: busybox <busybox@...ybox.net>, musl@...ts.openwall.com
Subject: Re: [PATCH] linedit, deluser: use POSIX getpwent instead of
 getpwent_r

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@...c.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

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.