This is the mail archive of the cygwin-developers@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: vfscanf in newlib


A __cursory__ conde read through gives me the impression that
simultaneous calls with the same file may result in undefined results.

buffer wise it should be ok, but don't use the same handle twice. I
don't know that the SUS has to say about the _r function w.r.t. the file
in use, but I would expect that two calls from two threads to one of the
_r functions should result in one blocking until the other finishes
scnaning, not them interleaving fread calls.

I may be completely off-base here though.

Rob


----- Original Message -----
From: "Charles Wilson" <cwilson@ece.gatech.edu>
To: <cygwin-developers@cygwin.com>
Cc: <cygwin-developers@cygwin.com>; <jjohnstn@cygnus.com>
Sent: Saturday, April 21, 2001 2:12 PM
Subject: Re: vfscanf in newlib


> Charles Wilson wrote:
> >
> > Christopher Faylor wrote:
> > > >J Johnston wrote:
> > > >As discussed, I have attached the new patch.  I moved stuff
around in stdio.h
> > > >because there were multiple sections using the same #ifdef.
There were also
> > > >some routines that were not properly under the non-strict ANSI
flag.
> > >
> > > I'm not sure if this is directed to me, but I was just asking
Chuck for the
> > > appropriate cygwin.din changes.
> > >
> > > I decided that it was silly for me to ask him to do this since it
is easy to
> > > do myself, though, so I've made the appropriate modifications.
So, if/when you
> > > check this in we'll be ready.
> >
> > Great -- thanks for taking care of that.  One comment, though:
> >
> > if the undecorated symbol is
> >   _vscanf_r
> > then the decorated symbol should be
> >  __vscanf_r
> >
> > So, shouldn't cygwin.din have
> > _vscanf_r
> > __vscanf_r = _vscanf_r
> >
> > Your patch has
> > _vscanf_f
> > vscanf_r = _vscanf_r
>
> [NOTE to newlib folks: the cygwin.din commentary above is
> cygwin-specific, ignore]
>
> Regardless of any argument about cygwin.din and cygwin1.dll exports, I
> built a new cygwin kernel using Jeff's most recent vfscanf patch
(taken
> via 'cvs update' since he'd already committed it before I got a chance
> to test).  Also, my new kernel used Chris Faylor's changes to
> cygwin.din.
>
> I snarfed some test programs from the web, and built against the new
> cygwin1.dll -- and it all worked fine.  I tested (briefly):
>   vfprintf
>   vprintf
>   vsprintf
>   vsnprintf
>   vfscanf
>   vscanf
>   vsscanf
>
> "My" test package is attached. Unpack, make, make check.
>
> I did not test any of the reentrant functions directly.
>
> In the case of _vfscanf_r, _vscanf_r, and _vsscanf_r, these three
> reentrant functions call the same core workhorse function that the
> "non-reentrant" functions do: __svfscanf_r().  So, the reentrant
> functions should *work*, but I can't guarantee they are truly
reentrant.
>
> In the case of the reentrant vfprintf derivatives, a few notes:
>
>   vfprintf calls _vfprintf_r {<--- THIS IS OK}
>     fine.  testing vfprintf tests _vfprintf_r (but doesn't *prove*
that
> _vfprintf_r is reentrant)
>
> -----
>
>   vprintf calls vfprintf (which calls _vfprintf_r AFTER doing a
> CHECK_INIT()) {THIS IS OK}.  vprintf is not declared using _DEFUN.
>   there is no _vprintf_r
>
> -----
>
>   vsprintf calls vfprintf (which calls _vfprintf_r AFTER ...) {<---
THIS
> IS OK, but...}
>   _vsprintf_r is misnamed "vsprintf_r" (no leading underscore), but
the
> header files declare it *with* the underscore.
>   Neither function is declared with _DEFUN.
>   Also, "vsprintf_r" calls vfprintf, not _vfprintf_r.  Shouldn't it
call
> the reentrant base function?  I know that vfprintf calls _vfprintf_r,
> but it does so only after calling CHECK_INIT on the faked-up file
> discriptor.
>
>   CHECK_INIT sets the ->_data field of the fake filedesc to "_REENT",
> but _vsprintf_r already sets that field explicitly to the reentrancy
> pointer originally passed in to _vsprintf_r ?  CHECK_INIT also
performs
> some tests (and possibly initialization) on the __sdidinit field of
the
> reentrancy pointer -- this is probably unnecessary if you set fp->data
=
> ptr.
>
> -----
>
>   vsnprintf calls vfprintf (which calls _vfprintf_r AFTER ...) {<---
> THIS IS OK, but ...}
>   _vsnprintf_r is misnamed "vsnprintf_r" (no leading underscore).
Also,
> _vsnprintf_r calls vfprintf instead of _vfprintf_r. Neither function
is
> declared with _DEFUN.
>
> -----
>
> If my arguments are correct, a patch to implement the required changes
> is attached.  I've rebuilt cygwin1.dll (*again*) with these patches,
and
> run my test suite (*again*) also against this new(er)
> cygwin1.dll/newlib.
>
> --Chuck
>
> ChangeLog
>
> Fri Apr 20 23:48:00 2001  Charles Wilson  <cwilson@ece.gatech.edu
>
> * libc/stdio/vprintf.c (vprintf): fix signature to use _DEFUN
> * libc/stdio/vprintf.c (_vprintf_r): new function
> * libc/stdio/vsnprintf.c (vsnprintf): fix signature to use _DEFUN
> * libc/stdio/vsnprintf.c (_vsnprintf_r): fix signature to use
> _DEFUN, and call _vfprintf_r, not vfprintf.
> * libc/stdio/vsprintf.c (vsprintf.c): fix signature to use _DEFUN
> * libc/stdio/vsnprintf.c (_vsprintf_r): fix signature to use
> _DEFUN, and call _vfprintf_r, not vfprintf.


------------------------------------------------------------------------
--------


> Index: libc/stdio/vprintf.c
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/stdio/vprintf.c,v
> retrieving revision 1.1.1.1
> diff -u -r1.1.1.1  old/libc/stdio/vprintf.c new/libc/stdio/vprintf.c
> --- old/libc/stdio/vprintf.c 2000/02/17 19:39:47 1.1.1.1
> +++ new/libc/stdio/vprintf.c 2001/04/21 03:43:55
> @@ -27,9 +27,18 @@
>  #endif
>
>  int
> -vprintf (fmt, ap)
> -     char _CONST *fmt;
> -     va_list ap;
> +_DEFUN (vprintf, (fmt, ap),
> +     _CONST char *fmt _AND
> +     va_list ap)
>  {
>    return vfprintf (stdout, fmt, ap);
> +}
> +
> +int
> +_DEFUN (_vprintf_r, (ptr, fmt, ap),
> +     struct _reent *ptr _AND
> +     _CONST char *fmt _AND
> +     va_list ap)
> +{
> +  return _vfprintf_r (ptr, _stdout_r (ptr), fmt, ap);
>  }
> Index: libc/stdio/vsnprintf.c
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/stdio/vsnprintf.c,v
> retrieving revision 1.2
> diff -u -r1.2  old/libc/stdio/vsnprintf.c new/libc/stdio/vsnprintf.c
> --- old/libc/stdio/vsnprintf.c 2000/08/08 19:01:02 1.2
> +++ new/libc/stdio/vsnprintf.c 2001/04/21 03:43:55
> @@ -34,11 +34,11 @@
>  #endif
>
>  int
> -vsnprintf (str, size, fmt, ap)
> -     char *str;
> -     size_t size;
> -     char _CONST *fmt;
> -     va_list ap;
> +_DEFUN (vsnprintf, (str, size, fmt, ap),
> +     char *str _AND
> +     size_t size _AND
> +     _CONST char *fmt _AND
> +     va_list ap)
>  {
>    int ret;
>    FILE f;
> @@ -54,12 +54,12 @@
>  }
>
>  int
> -vsnprintf_r (ptr, str, size, fmt, ap)
> -     struct _reent *ptr;
> -     char *str;
> -     size_t size;
> -     char _CONST *fmt;
> -     va_list ap;
> +_DEFUN (_vsnprintf_r, (ptr, str, size, fmt, ap),
> +     struct _reent *ptr _AND
> +     char *str _AND
> +     size_t size _AND
> +     _CONST char *fmt _AND
> +     va_list ap)
>  {
>    int ret;
>    FILE f;
> @@ -68,7 +68,7 @@
>    f._bf._base = f._p = (unsigned char *) str;
>    f._bf._size = f._w = (size > 0 ? size - 1 : 0);
>    f._data = ptr;
> -  ret = vfprintf (&f, fmt, ap);
> +  ret = _vfprintf_r (ptr, &f, fmt, ap);
>    if (size > 0)
>      *f._p = 0;
>    return ret;
> Index: libc/stdio/vsprintf.c
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/stdio/vsprintf.c,v
> retrieving revision 1.1.1.1
> diff -u -r1.1.1.1  old/libc/stdio/vsprintf.c new/libc/stdio/vsprintf.c
> --- old/libc/stdio/vsprintf.c 2000/02/17 19:39:47 1.1.1.1
> +++ new/libc/stdio/vsprintf.c 2001/04/21 03:43:57
> @@ -32,10 +32,10 @@
>  #endif
>
>  int
> -vsprintf (str, fmt, ap)
> -     char *str;
> -     char _CONST *fmt;
> -     va_list ap;
> +_DEFUN (vsprintf, (str, fmt, ap),
> +     char *str _AND
> +     _CONST char *fmt _AND
> +     va_list ap)
>  {
>    int ret;
>    FILE f;
> @@ -50,11 +50,11 @@
>  }
>
>  int
> -vsprintf_r (ptr, str, fmt, ap)
> -     struct _reent *ptr;
> -     char *str;
> -     char _CONST *fmt;
> -     va_list ap;
> +_DEFUN (_vsprintf_r, (ptr, str, fmt, ap),
> +     struct _reent *ptr _AND
> +     char *str _AND
> +     _CONST char *fmt _AND
> +     va_list ap)
>  {
>    int ret;
>    FILE f;
> @@ -63,7 +63,7 @@
>    f._bf._base = f._p = (unsigned char *) str;
>    f._bf._size = f._w = INT_MAX;
>    f._data = ptr;
> -  ret = vfprintf (&f, fmt, ap);
> +  ret = _vfprintf_r (ptr, &f, fmt, ap);
>    *f._p = 0;
>    return ret;
>  }
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]