[PATCH] Add getmntent_r

Corinna Vinschen corinna-cygwin@cygwin.com
Tue Jun 5 12:42:00 GMT 2012


Hi Yaakov,

thanks for the patch, but this won't fly.

On Jun  5 00:08, Yaakov (Cygwin/X) wrote:
> This patch set implements getmntent_r, a GNU extension:
> 
> http://man7.org/linux/man-pages/man3/getmntent.3.html
> 
> libvirt needs this[1], as I just (re)discovered.  Patches for
> winsup/cygwin and winsup/doc attached.

> +extern "C" struct mntent *
> +getmntent_r (FILE *, struct mntent *mntbuf, char *buf, int buflen)
> +{
> +  struct mntent *mnt = mount_table->getmntent (_my_tls.locals.iteration++);
> +  char *tmpbuf;
> +  int len = 0, maxlen;
> +
> +  if (!mnt)
> +    {
> +      mntbuf = NULL;

This doesn't make sense since mntbuf is a local varibale.  Changing
its value won't be propagated by the calling function anyway.

> +      return NULL;
> +    }
> +
> +  maxlen = strlen (mnt->mnt_fsname) + strlen (mnt->mnt_dir)
> +           + strlen (mnt->mnt_type) + strlen (mnt->mnt_opts) + 30;
> +  tmpbuf = (char *) alloca (maxlen);
> +  memset (tmpbuf, '\0', maxlen);
> +
> +  len += __small_sprintf (tmpbuf, "%s", mnt->mnt_fsname) + 1;
> +  len += __small_sprintf (tmpbuf + len, "%s", mnt->mnt_dir) + 1;
> +  len += __small_sprintf (tmpbuf + len, "%s", mnt->mnt_type) + 1;
> +  len += __small_sprintf (tmpbuf + len, "%s", mnt->mnt_opts) + 1;

This you can have simpler.

> +  len += __small_sprintf (tmpbuf + len, "%d %d", mnt->mnt_freq, mnt->mnt_passno);

and this I don't grok at all.  Why don't you just copy over the
numbers from mnt to mntbuf?

> +
> +  memcpy (buf, tmpbuf, buflen);

So the function returns success even if the incoming buffer space
is insufficient?

> +  memcpy (mntbuf, mnt, sizeof (struct mntent));

This doesn't do what you want.  mntbuf is just a copy of mnt, so the
mntbuf members won't point to buf, but they will point to the internal
storage.  While you have made a copy of the internal strings, they won't
be used.  Also, assuming you first store the strings in tmpbuf and then
memcpy them over to buf, 

> +  return mnt;

And this returns the internal mntent entry anyway, so even mntbuf is
kind of moot.  What you want is more like this (untested):

  struct mntent *mnt = mount_table->getmntent (_my_tls.locals.iteration++);
  if (!mnt)
    return NULL;
   int maxlen = strlen (mnt->mnt_fsname) + strlen (mnt->mnt_dir)
		+ strlen (mnt->mnt_type) + strlen (mnt->mnt_opts) + 4;
   if (maxlen > buflen)
     return NULL;
   mntbuf->mnt_dir = stpcpy (mntbuf->mnt_fsname = buf, mnt->mnt_fsname);
   mntbuf->mnt_type = stpcpy (mntbuf->mnt_dir, mnt->mnt_dir);
   mntbuf->mnt_opts = stpcpy (mntbuf->mnt_type, mnt->mnt_type);
   mntbuf->mnt_freq = mnt->mnt_freq;
   mntbuf->mnt_passno = mnt->mnt_passno;
   memcpy (buf, tmpbuf, buflen);
   return mntbuf;


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat



More information about the Cygwin-patches mailing list