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. Yaakov [1] http://cygwin.com/ml/cygwin/2010-04/msg00885.html
On 2012-06-05 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. And here is the code I used to test on Cygwin and Linux. Yaakov
#ifdef CCOD #pragma CCOD:script no #endif #include <mntent.h> #include <stdlib.h> #include <stdio.h> #include <string.h> #ifdef __CYGWIN__ #include <dlfcn.h> #include <cygwin/version.h> #endif int main(void)(Continue reading)
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)(Continue reading)
On Jun 5 14:42, Corinna Vinschen wrote: > 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 ^^ variable > its value won't be propagated by the calling function anyway. ^^(Continue reading)
And I missed a line in my dirty code example:
On Jun 5 14:42, Corinna Vinschen wrote:
> 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);
stpcpy (mntbuf->mnt_opts, mnt->mnt_opts);
> 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
(Continue reading)
On Jun 5 16:39, Corinna Vinschen wrote:
> And I missed a line in my dirty code example:
And I missed to add a + 1 to the pointers returned from stpcpy. Sigh.
Let's restart:
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_fsname = buf;
mntbuf->mnt_dir = stpcpy (mntbuf->mnt_fsname, mnt->mnt_fsname) + 1;
mntbuf->mnt_type = stpcpy (mntbuf->mnt_dir, mnt->mnt_dir) + 1;
mntbuf->mnt_opts = stpcpy (mntbuf->mnt_type, mnt->mnt_type) + 1;
stpcpy (mntbuf->mnt_opts, mnt->mnt_opts);
mntbuf->mnt_freq = mnt->mnt_freq;
mntbuf->mnt_passno = mnt->mnt_passno;
memcpy (buf, tmpbuf, buflen);
return mntbuf;
Sorry about that. Hopefully you get the idea, regardless.
Corinna
--
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Project Co-Leader cygwin AT cygwin DOT com
(Continue reading)
On 2012-06-05 07:42, Corinna Vinschen wrote:
>> +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.
Further testing of glibc shows that buf and mntbuf are indeed left
untouched when returning 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.
>
(Continue reading)
On Jun 5 21:29, Yaakov (Cygwin/X) wrote:
> On 2012-06-05 07:42, Corinna Vinschen wrote:
> >>+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.
>
> Further testing of glibc shows that buf and mntbuf are indeed left
> untouched when returning NULL.
Erm... I'm not talking about glibc vs. your code. Mntbuf is a simple
call-by-value variable. Changes to this variable in the called function
are *never* propagated back to the parent function.
> >>+ 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;
(Continue reading)
Somehow this one fell through the cracks. Picking up where we left off: On 2012-06-06 02:33, Corinna Vinschen wrote: > On Jun 5 21:29, Yaakov (Cygwin/X) wrote: >> The string returned into buf is in the following format: >> >> mnt_fsname\0mnt_dir\0mnt_type\0mnt_opts\0mnt_freq" "mnt_passno\0 > > Yes, but this is not something inherent to the functionality of > getmntent_r, it's just residue from the way getmntent_r works. It reads > a line from /etc/fstab or /etc/mtab into the buffer via fgets: > > mnt_fsname\tmnt_dir\tmnt_type\tmnt_opts\tmnt_freq\n > > and then creates the content of mntbuf from there, replacing the \t with > \0 as it goes along. So it starts with mnt_opts and mnt_freq strings in > buf, but only to sscanf them into their respective mntbuf->mnt_opts and > mntbuf->mnt_freq members. Since glibc is getting this information from the kernel, that makes sense. > In case of Cygwin this is not needed since we don't read from the file > but from the internal datastructure. There's no reason to create > garbage in buf just because this is by chance the layout the buffer gets > when operating under Linux. > > The *important* thing is that buf contains the strings the members of > mntbuf points to. OK, revised patch attached.(Continue reading)
On Jul 18 05:21, Yaakov (Cygwin/X) wrote: > Somehow this one fell through the cracks. Picking up where we left off: > > On 2012-06-06 02:33, Corinna Vinschen wrote: > >On Jun 5 21:29, Yaakov (Cygwin/X) wrote: > >>The string returned into buf is in the following format: > >> > >>mnt_fsname\0mnt_dir\0mnt_type\0mnt_opts\0mnt_freq" "mnt_passno\0 > > > >Yes, but this is not something inherent to the functionality of > >getmntent_r, it's just residue from the way getmntent_r works. It reads > >a line from /etc/fstab or /etc/mtab into the buffer via fgets: > > > > mnt_fsname\tmnt_dir\tmnt_type\tmnt_opts\tmnt_freq\n > > > >and then creates the content of mntbuf from there, replacing the \t with > >\0 as it goes along. So it starts with mnt_opts and mnt_freq strings in > >buf, but only to sscanf them into their respective mntbuf->mnt_opts and > >mntbuf->mnt_freq members. > > Since glibc is getting this information from the kernel, that makes sense. > > >In case of Cygwin this is not needed since we don't read from the file > >but from the internal datastructure. There's no reason to create > >garbage in buf just because this is by chance the layout the buffer gets > >when operating under Linux. > > > >The *important* thing is that buf contains the strings the members of > >mntbuf points to. >(Continue reading)
On 2012-07-18 06:17, Corinna Vinschen wrote: > On Jul 18 05:21, Yaakov (Cygwin/X) wrote: >> On 2012-06-06 02:33, Corinna Vinschen wrote: >>> In case of Cygwin this is not needed since we don't read from the file >>> but from the internal datastructure. There's no reason to create >>> garbage in buf just because this is by chance the layout the buffer gets >>> when operating under Linux. >>> >>> The *important* thing is that buf contains the strings the members of >>> mntbuf points to. >> >> OK, revised patch attached. > > Thanks. Applied with a tweak: It's really not necessary at all to > create strings for mnt_freq and mnt_passno in buf. Just copy them > over from mnt to mntbuf and be done with it. In that case, we don't need opts_len, and AFAICS it will introduce a warning with GCC 4.6 [-Wunused-but-set-variable]. OK to remove? Yaakov
On Jul 18 15:23, Yaakov (Cygwin/X) wrote: > On 2012-07-18 06:17, Corinna Vinschen wrote: > >On Jul 18 05:21, Yaakov (Cygwin/X) wrote: > >>On 2012-06-06 02:33, Corinna Vinschen wrote: > >>>In case of Cygwin this is not needed since we don't read from the file > >>>but from the internal datastructure. There's no reason to create > >>>garbage in buf just because this is by chance the layout the buffer gets > >>>when operating under Linux. > >>> > >>>The *important* thing is that buf contains the strings the members of > >>>mntbuf points to. > >> > >>OK, revised patch attached. > > > >Thanks. Applied with a tweak: It's really not necessary at all to > >create strings for mnt_freq and mnt_passno in buf. Just copy them > >over from mnt to mntbuf and be done with it. > > In that case, we don't need opts_len, and AFAICS it will introduce a > warning with GCC 4.6 [-Wunused-but-set-variable]. OK to remove? Sure! Thanks, Corinna -- -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Project Co-Leader cygwin AT cygwin DOT com Red Hat(Continue reading)
RSS Feed