Hi Vladimir,
Vladimir Kirillov wrote on Fri, Oct 15, 2010 at 02:31:21AM +0300:
> The getmntinfo(3) page says that the mentioned function uses static
> storage which cannot be freed, however this storage is being actually
> malloced and can be freed without consequences.
Err, no.
Reading the code below, in case you free the pointer returned
in *mntbufp, the address will still exist internally in the static
struct statfs *mntbuf, and the next time you call getmntinfo(),
the statement if (mntbuf) free(mntbuf) results in a double free.
> I see no real need in doing such weird things (correct me if i'm
> wrong)
As far as i understand, getmntinfo() cannot use a static buffer
because the size is not known beforehand. Thus, it mallocs
its internal buffer and frees it during the next call.
Off the top of my head, i wouldn't know how to simplify that.
> so here's a diff to actually do pass a singly-malloced buffer
> to the caller who should free it manually.
No, wait, you can't change the calling convention of an existing
library function just like that. That would turn all application
code which is now correct into a horrible bunch of memory leaks,
and all application code written for the new convention would
result in double frees when linked against old libraries.
> I've also converted the essential users of that function:
> bin/df sbin/mount sbin/umount sbin/mountd usr.bin/fstat.
> If you find this approach good/useful I'll convert the rest (the
> manpage will also need some love).
And what about those users of the function not in base?
This is a library function, anybody may be using it in private code.
> It is good to subsequently replace all static stuff in libc, right?
Not quite.
Instead, some people double the number of libc functions by
adding thread-safe versions, like in ctime() -> ctime() and ctime_r(),
and that is ugly enough.
I'm pretty sure you must not change how existing functions work.
Yours,
Ingo
> Index: getmntinfo.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/gen/getmntinfo.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 getmntinfo.c
> --- getmntinfo.c 8 Aug 2005 08:05:34 -0000 1.7
> +++ getmntinfo.c 14 Oct 2010 22:28:06 -0000
> @@ -38,25 +38,22 @@
> int
> getmntinfo(struct statfs **mntbufp, int flags)
> {
> - static struct statfs *mntbuf;
> - static int mntsize;
> - static size_t bufsize;
> + struct statfs *mntbuf;
> + int mntsize;
> + size_t bufsize = 0;
>
> - if (mntsize <= 0 && (mntsize = getfsstat(0, 0, MNT_NOWAIT)) < 0)
> + if ((mntsize = getfsstat(NULL, 0, MNT_NOWAIT)) < 0)
> return (0);
> - if (bufsize > 0 && (mntsize = getfsstat(mntbuf, bufsize, flags)) < 0)
> +
> + bufsize = mntsize * sizeof(struct statfs);
> + if ((mntbuf = malloc(bufsize)) == NULL)
> + return (0);
> +
> + if ((mntsize = getfsstat(mntbuf, bufsize, flags)) < 0) {
> + free(mntbuf);
> return (0);
> - while (bufsize <= mntsize * sizeof(struct statfs)) {
> - if (mntbuf)
> - free(mntbuf);
> - bufsize = (mntsize + 1) * sizeof(struct statfs);
> - if ((mntbuf = (struct statfs *)malloc(bufsize)) == 0) {
> - bufsize = 0;
> - return (0);
> - }
> - if ((mntsize = getfsstat(mntbuf, bufsize, flags)) < 0)
> - return (0);
> }
> +
> *mntbufp = mntbuf;
> return (mntsize);
> }
