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

[Alejandro Colomar]

On Mon, Sep 29, 2025 at 01:00:09AM +0200, Alejandro Colomar wrote:
> 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;

I forgot a p++ here.

> 
> 		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/>).



-- 
<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/22f7994e/attachment.sig>