Date: Wed, 6 Jan 2010 21:30:08 GMT From: John Baldwin <jhb@freebsd.org> To: freebsd-threads@FreeBSD.org Subject: Re: threads/141198: src/lib/libc/stdio does not properly initialize mutexes Message-ID: <201001062130.o06LU87Q010835@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR threads/141198; it has been noted by GNATS.
From: John Baldwin <jhb@freebsd.org>
To: freebsd-threads@freebsd.org
Cc: freebsd-gnats-submit@freebsd.org,
Jeremy Huddleston <jeremyhu@apple.com>
Subject: Re: threads/141198: src/lib/libc/stdio does not properly initialize mutexes
Date: Wed, 6 Jan 2010 16:00:47 -0500
On Monday 07 December 2009 8:50:37 am John Baldwin wrote:
> On Saturday 05 December 2009 3:34:41 pm Jeremy Huddleston wrote:
> >
> > >Number: 141198
> > >Category: threads
> > >Synopsis: src/lib/libc/stdio does not properly initialize mutexes
> > >Confidential: no
> > >Severity: non-critical
> > >Priority: low
> > >Responsible: freebsd-threads
> > >State: open
> > >Quarter:
> > >Keywords:
> > >Date-Required:
> > >Class: sw-bug
> > >Submitter-Id: current-users
> > >Arrival-Date: Sat Dec 05 20:40:01 UTC 2009
> > >Closed-Date:
> > >Last-Modified:
> > >Originator: Jeremy Huddleston
> > >Release: 8.0
> > >Organization:
> > Apple
> > >Environment:
> > NA
> > >Description:
> > libc/stdio assumes PTHREAD_MUTEX_INITIALIZER is NULL (which it is in
> FreeBSD), but this makes the code not as portable.
> >
> > Earlier versions of stdio did properly initialize the lock to
> PTHREAD_MUTEX_INITIALIZER in INITEXTRA() when it was part of the _extra
> extension substructure.
>
> This is my fault. I suspect it was more an error of omission on my part than
> assuming the default value of PTHREAD_MUTEX_INITIALIZER. :) Hmm, so I
> reviewed the change that removed INITEXTRA() and all the places it was used to
> construct a 'fake' FILE object it was passed to an internal stdio routine that
> did not use locking. One thing I do see that is an old bug is that the three
> static FILE structures used for stdin, stdout, and stderr have never had their
> internal locks properly initialized. Also, it seems __sfp() never initializes
> fp->_lock at all. Oh, it gets set to NULL via 'empty' in moreglue(). That is
> also an old bug. I think this will fix those problems:
>
> Index: stdio/findfp.c
> ===================================================================
> --- stdio/findfp.c (revision 199969)
> +++ stdio/findfp.c (working copy)
> @@ -61,6 +61,7 @@
> ._read = __sread, \
> ._seek = __sseek, \
> ._write = __swrite, \
> + ._fl_mutex = PTHREAD_MUTEX_INITIALIZER, \
> }
> /* the usual - (stdin + stdout + stderr) */
> static FILE usual[FOPEN_MAX - 3];
> @@ -96,7 +97,7 @@
> int n;
> {
> struct glue *g;
> - static FILE empty;
> + static FILE empty = { ._fl_mutex = PTHREAD_MUTEX_INITIALIZER };
> FILE *p;
>
> g = (struct glue *)malloc(sizeof(*g) + ALIGNBYTES + n * sizeof(FILE));
> @@ -154,7 +155,7 @@
> fp->_ub._size = 0;
> fp->_lb._base = NULL; /* no line buffer */
> fp->_lb._size = 0;
> -/* fp->_lock = NULL; */ /* once set always set (reused) */
> +/* fp->_fl_mutex = NULL; */ /* once set always set (reused) */
> fp->_orientation = 0;
> memset(&fp->_mbstate, 0, sizeof(mbstate_t));
> return (fp);
Does this patch address the concerns?
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201001062130.o06LU87Q010835>