sfio: make 'Sfio_t' definition entirely internal

The definition of `Sfio_t` was structured to give it a public part and a private
part:

  struct _sfio_s {
    … public members …

    _SFIO_PRIVATE; // ← only visible if sfio_t.h is included
  };

This works only as long as no user ever allocates a `Sfio_t` themselves. If they
do, they will allocate too few bytes, based on their view of the size of
`Sfio_t` missing its private members.

A side effect of link-time optimization is that the compiler can see through
this trickery and witness both with-private and without-private definitions at
once. Creating objects of this type or pointers to objects of this type is a
violation of C’s strict aliasing rule and the compiler can now see this. This
can cause the compiler to make unsafe optimizations, like concluding any code
involving `Sfio_t*` variables must be unreachable.

The safer way to do a public/private class like this is with two structs, the
private one containing the public one as a member:

  struct foo_public {
    … public members …
  };

  struct foo_private {
    struct foo_public *public;

    … private members …
  };

Public API functions then accept `struct foo_public*` parameters and internally
convert them to `struct foo_private*` variables in order to access the
internals:

  int foo_do_something(struct foo_public *foo) {
    // use something like the Linux kernel’s container_of
    struct foo_private *f = container_of(foo, public);
    …
  }

But instead of this, we can observe that the definition of `Sfio_t` does not
need to have any public members at all, and we can make the entire type private.
We need an exception for libexpr which reaches into sfio internals.

Gitlab: fixes #2338

diff --git a/lib/expr/exlib.h b/lib/expr/exlib.h
index 1d5871717a123180141966a5a2367134d57f0c3e..2415e76704c80bf744d5b8ee02b6625512c83c37 100644 (file)
--- a/lib/expr/exlib.h
+++ b/lib/expr/exlib.h
@@ -23,6 +23,7 @@ extern "C" {
 #define _EXLIB_H
 
 #include <ast/ast.h>
+#include <sfio/sfio_t.h>
 
 #define sfstrseek(f,p,m) \
     ( \

diff --git a/lib/sfio/sfio.h b/lib/sfio/sfio.h
index 7a43318425dc774463ffb67e99f98531e45469ca..ab92cc84c7541271ffeabfd0b5aeda7a4243c914 100644 (file)
--- a/lib/sfio/sfio.h
+++ b/lib/sfio/sfio.h
@@ -56,23 +56,6 @@ extern "C" {
        Sfdisc_t *disc;         /* the continuing discipline    */
     };
 
-/* a file structure */
-    struct _sfio_s {
-       unsigned char *next;    /* next position to read/write from     */
-       unsigned char *endw;    /* end of write buffer                  */
-       unsigned char *endr;    /* end of read buffer                   */
-       unsigned char *endb;    /* end of buffer                        */
-       Sfio_t *push;           /* the stream that was pushed on        */
-       unsigned short flags;   /* type of stream                       */
-       short file;             /* file descriptor                      */
-       unsigned char *data;    /* base of data buffer                  */
-       ssize_t size;           /* buffer size                          */
-       ssize_t val;            /* values or string lengths             */
-#ifdef _SFIO_PRIVATE
-        _SFIO_PRIVATE
-#endif
-    };
-
 /* formatting environment */
     typedef struct _sffmt_s Sffmt_t;
     typedef int (*Sffmtext_f)(void *, Sffmt_t *);

diff --git a/lib/sfio/sfio_t.h b/lib/sfio/sfio_t.h
index 3e0a914b3e38c31ccd4f07403220fe1d3ed11cd7..0f5f5eeb92bcf4d205ff93f3a40f3894ba84723f 100644 (file)
--- a/lib/sfio/sfio_t.h
+++ b/lib/sfio/sfio_t.h
@@ -38,6 +38,23 @@ extern "C" {
 
 #include       <sfio/sfio.h>
 
+/* a file structure */
+    struct _sfio_s {
+       unsigned char *next;    /* next position to read/write from     */
+       unsigned char *endw;    /* end of write buffer                  */
+       unsigned char *endr;    /* end of read buffer                   */
+       unsigned char *endb;    /* end of buffer                        */
+       Sfio_t *push;           /* the stream that was pushed on        */
+       unsigned short flags;   /* type of stream                       */
+       short file;             /* file descriptor                      */
+       unsigned char *data;    /* base of data buffer                  */
+       ssize_t size;           /* buffer size                          */
+       ssize_t val;            /* values or string lengths             */
+#ifdef _SFIO_PRIVATE
+        _SFIO_PRIVATE
+#endif
+    };
+
 /* mode bit to indicate that the structure hasn't been initialized */
 #define SF_INIT                0000004u