From: Matthew Fernandez <matthew.fernandez@gmail.com>
Date: Thu, 19 Jan 2023 16:24:02 +0000 (-0800)
Subject: sfio: make 'Sfio_t' definition entirely internal
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=99c2ef867f43ef7ee60190f55c723d74ff310512;p=graphviz

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 1d5871717..2415e7670 100644
--- 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 7a4331842..ab92cc84c 100644
--- 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 3e0a914b3..0f5f5eeb9 100644
--- 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