sfio: remove 'SF_MTSAFE'

This constant was performing two overlapping roles:

  1. A flag callers could use to indicate they anticipated calling into sfio
     functions with the same object from multiple threads.

  2. A flag used by the library itself to indicate whether thread-safety was
     available.

To deal with both macros having the same name, sfhdr.h was forcing the value
seen internally to zero. This mechanism has numerous shortcomings:

  1. It was possible for a caller to use the external value of `SF_MTSAFE`
     (010000) that, thanks to the overriding described above, sfio itself would
     fail to understand.

  2. Including sfio.h within sfio sources without including sfhdr.h was a latent
     foot gun that would result in part of sfio believing the `SF_MTSAFE` flag
     _did_ have meaning.

  3. sfio functions are not thread-safe, with or without `SF_MTSAFE` defined,
     and regardless of what value it has.

To take an example to illustrate 3, `sfopen` does something like double-checked
locking¹ to anticipate another thread modifying `f` in-between its first check
and the point at which it calls `SFMTXSTART`. Except `SFMTXSTART` is not a lock.
There is no synchronization point here, and in fact an optimizing compiler will
simply coalesce both the first and second checks into one. It is not clear to me
whether this code was written when multicore machines were uncommon (though this
design is also unsafe on a unicore machine) or if `SFMTXSTART` is some kind of
placeholder that was meant to one day grow a locking mechanism.

It seems safer and more honest to remove this constant, and with it the
implication that thread-safety is toggle-able. Migrating to C stdio (#1998) will
render all of this moot as it is thread-safe by default.

¹ https://en.wikipedia.org/wiki/Double-checked_locking

diff --git a/lib/sfio/sfextern.c b/lib/sfio/sfextern.c
index fcf4f3992bc6e599b041f26e65dbbf1a087d0493..bf7ef301ab5f42404b4e466ae6c5269fae9b4787 100644 (file)
--- a/lib/sfio/sfextern.c
+++ b/lib/sfio/sfextern.c
@@ -25,12 +25,9 @@ ssize_t _Sfi = -1;
 #define SFMTXOUT       (0)
 #define SFMTXERR       (0)
 
-Sfio_t _Sfstdin = SFNEW(NULL, -1, 0, (SF_READ | SF_STATIC | SF_MTSAFE), NULL,
-                       SFMTXIN);
-Sfio_t _Sfstdout = SFNEW(NULL, -1, 1, (SF_WRITE | SF_STATIC | SF_MTSAFE), NULL,
-                       SFMTXOUT);
-Sfio_t _Sfstderr = SFNEW(NULL, -1, 2, (SF_WRITE | SF_STATIC | SF_MTSAFE), NULL,
-                       SFMTXERR);
+Sfio_t _Sfstdin = SFNEW(NULL, -1, 0, (SF_READ | SF_STATIC), NULL, SFMTXIN);
+Sfio_t _Sfstdout = SFNEW(NULL, -1, 1, (SF_WRITE | SF_STATIC), NULL, SFMTXOUT);
+Sfio_t _Sfstderr = SFNEW(NULL, -1, 2, (SF_WRITE | SF_STATIC), NULL, SFMTXERR);
 
 Sfio_t *sfstdin = &_Sfstdin;
 Sfio_t *sfstdout = &_Sfstdout;

diff --git a/lib/sfio/sfhdr.h b/lib/sfio/sfhdr.h
index 745d29e9fd4942a028f02de32e286f9bbece5a88..0700a4f31194a8badb672ce36cbda395a5aaea60 100644 (file)
--- a/lib/sfio/sfhdr.h
+++ b/lib/sfio/sfhdr.h
@@ -57,9 +57,6 @@ extern "C" {
 #include       <errno.h>
 #include       <ctype.h>
 
-#undef SF_MTSAFE               /* no need to worry about thread-safety */
-#define SF_MTSAFE              0
-
 #define SFMTXSTART(f,v)                { if(!f) return(v); }
 #define SFMTXRETURN(f,v)       { return(v); }
 
@@ -360,8 +357,8 @@ extern "C" {
 /* lock/open a stream */
 #define SFMODE(f,l)    ((f)->mode & ~(SF_RV|SF_RC|((l) ? SF_LOCK : 0)) )
 #define SFLOCK(f,l)    (void)((f)->mode |= SF_LOCK, (f)->endr = (f)->endw = (f)->data)
-#define _SFOPENRD(f)   ((f)->endr = ((f)->flags&SF_MTSAFE) ? (f)->data : (f)->endb)
-#define _SFOPENWR(f)   ((f)->endw = ((f)->flags&(SF_MTSAFE|SF_LINE)) ? (f)->data : (f)->endb)
+#define _SFOPENRD(f)   ((f)->endr = (f)->endb)
+#define _SFOPENWR(f)   ((f)->endw = ((f)->flags&SF_LINE) ? (f)->data : (f)->endb)
 #define _SFOPEN(f)     ((f)->mode == SF_READ  ? _SFOPENRD(f) : \
                         (f)->mode == SF_WRITE ? _SFOPENWR(f) : \
                         ((f)->endw = (f)->endr = (f)->data) )

diff --git a/lib/sfio/sfio.h b/lib/sfio/sfio.h
index b5bf21d5738b49f09531b826c75ac1d29dbe0dc6..3d6fb496a3d4af7a4999064c96e95185bc6081d7 100644 (file)
--- a/lib/sfio/sfio.h
+++ b/lib/sfio/sfio.h
@@ -136,7 +136,6 @@ extern "C" {
 #define SF_STATIC      0001000 /* a stream that cannot be freed        */
 #define SF_IOCHECK     0002000 /* call exceptf before doing IO         */
 #define SF_PUBLIC      0004000 /* SF_SHARE and follow physical seek    */
-#define SF_MTSAFE      0010000 /* need thread safety                   */
 #define SF_WHOLE       0020000 /* preserve wholeness of sfwrite/sfputr */
 
 #define SF_FLAGS       0077177 /* PUBLIC FLAGS PASSABLE TO SFNEW()     */

diff --git a/lib/sfio/sfopen.c b/lib/sfio/sfopen.c
index 2b83fa03098f13db61b31f7f44edb9411b5b48d5..d1dad6906f32486410ecc47cb6970f8b2d3e3726 100644 (file)
--- a/lib/sfio/sfopen.c
+++ b/lib/sfio/sfopen.c
@@ -161,11 +161,9 @@ int _sftype(const char *mode, int *oflagsp, int *uflagp)
                sflags |= SF_READ | SF_WRITE;
            continue;
        case 'm':
-           sflags |= SF_MTSAFE;
            uflag = 0;
            continue;
        case 'u':
-           sflags &= (unsigned short)~SF_MTSAFE;
            uflag = 1;
            continue;
        default: