getpwent_r(3) needs gr->gr_mem to be freed, but that's not documented

[Alejandro Colomar]

Hi Collin,

On Sun, Sep 28, 2025 at 03:43:54PM -0700, Collin Funk wrote:
> Alejandro Colomar <alx@kernel.org> writes:
> 
> > Hmmm, that's good.  Thanks!  At least, the user doesn't need to free(3)
> > anything weird.
> >
> > So, a good estimate of the size to be allocated prior to the
> > sgetgrent_r() call should be:
> >
> > 	size = strlen(s) + 1 + strchrcnt(s, ',') + 2;

Oops, actually:

	size = strlen(s) + 1;
	size += (strchrcnt(s, ',') + 2) * sizeof(char *);

> > That would be wasting a little bit if there are any commas outside of
> > the fourth ':'-delimited field, but it should work.
> 
> You can guess a value and then grow the buffer as long as errno == ERANGE.

TBH, I dislike that approach.  I prefer a good guess, and if that
doesn't work, I'll report an error.  It may be imperfect, but it's
simple, which means less bugs.  If I can have an upper bound, and the
wasted memory is reasonable (in this case, I'd say it's even
negligible), then we're fine.

So far, I've written this:

	// from-string get group entry
	struct group *
	sgetgrent(const char *s)
	{
		int                 e;
		size_t              size;
		static char         *buf = NULL;
		static struct group grent = {};

		free(buf);

		size = strlen(s) + 1;
		size += sizeof(char *) * (strchrcnt(s, ',') + 2);  // For 'grent.gr_mem'

		buf = MALLOC(size, char);
		if (buf == NULL)
			return NULL;

		e = sgetgrent_r(s, &grent, buf, size);
		if (e != 0) {
			errno = e;
			return NULL;
		}

		return &grent;
	}


	// from-string get group entry re-entrant
	int
	sgetgrent_r(size_t size;
	    const char *restrict s, struct group *restrict grent,
	    char buf[restrict size], size_t size)
	{
		char  *p, *end;
		char  *fields[4];

		end = buf + size;
		p = stpecpy(buf, end, s);
		if (p == NULL)
			return errno;

		stpsep(buf, "\n");

		if (STRSEP2ARR(buf, ":", fields) == -1)
			return EINVAL;

		grent->gr_name = fields[0];
		grent->gr_passwd = fields[1];
		if (get_gid(fields[2], &grent->gr_gid) == -1)
			return errno;

		grent->gr_mem = csv2ls(fields[3], end - p, p);
		if (NULL == grent->gr_mem)
			return errno;

		return 0;
	}

Which I'm quite happy with.  It's simple, it's loop-less (well, it has
some necessary loops, but it doesn't retry anything), and it should work
just fine.

> > BTW, where's this exactly in the glibc source code?  It's a bit hard to
> > follow.
> 
> In nss you can find <FUNCTION-NAME>.c and then follow the includes and
> macros. I agree it is a bit hard to follow. :)

Especially, if we start here (omitting the copyright notice, for
brevity):

	$ cat ./getgrent_r.c
	/* Copyright [...]  */

	#include <grp.h>


	#define LOOKUP_TYPE	struct group
	#define SETFUNC_NAME	setgrent
	#define	GETFUNC_NAME	getgrent
	#define	ENDFUNC_NAME	endgrent
	#define DATABASE_NAME	group
	#define BUFLEN		NSS_BUFLEN_GROUP

	#include "../nss/getXXent_r.c"

And it doesn't get much better once I start following that.  I think
I'll just trust Mark.  :-)


> Collin

Cheers,
Alex

-- 
<https://www.alejandro-colomar.es>
Use port 80 (that is, <...:80/>).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/libc-alpha/attachments/20250929/e0074a0d/attachment-0001.sig>