]> granicus.if.org Git - graphviz/commitdiff
GD plugin: remove repurposing of 'tell' member of I/O context
authorMatthew Fernandez <matthew.fernandez@gmail.com>
Thu, 3 Mar 2022 06:56:11 +0000 (22:56 -0800)
committerMatthew Fernandez <matthew.fernandez@gmail.com>
Sun, 13 Mar 2022 17:17:01 +0000 (10:17 -0700)
The GD plugin was using (abusing) the `gdIOCtx.tell` field as a way to pass
around the Graphviz context through GD library functions and callbacks. The GD
docs¹ have this to say about the `tell` field:

  The seek and tell functions are only required in conjunction with the gd2 file
  format, which supports quick loading of partial images.

The Graphviz GD plugin _is_ using gd2 functionality, so it seems its usage only
works out because it coincidentally does not hit any code paths that use `tell`.

We can avoid this latent issue entirely by rephrasing the way the plugin passes
the Graphviz context around to something less brittle. The approach taken in
this commit is inspired by something like Linux’s `container_of` macro.

¹ https://libgd.github.io/manuals/2.3.3/files/gd_io-h.html#gdIOCtx

plugin/gd/CMakeLists.txt
plugin/gd/Makefile.am
plugin/gd/gdioctx_wrapper.h [new file with mode: 0644]
plugin/gd/gvdevice_gd.c
plugin/gd/gvrender_gd.c

index 284254a6a52f9cac9e4ff1d1e67d9d3c9852bc2d..fbe38c8156b81d94fa676758596882e4d6e29c9d 100644 (file)
@@ -2,6 +2,7 @@ if(GD_FOUND)
 
   add_library(gvplugin_gd SHARED
     # Source files
+    gdioctx_wrapper.h
     gvdevice_gd.c
     gvloadimage_gd.c
     gvplugin_gd.c
index e56fbf249fc0c3db5fdc6bc7483af7a96572ed0b..d5d7f56cef05339a0442dd106a97569934ac7e40 100644 (file)
@@ -19,6 +19,7 @@ pkglib_LTLIBRARIES = libgvplugin_gd.la
 endif
 endif
 
+noinst_HEADERS = gdioctx_wrapper.h
 libgvplugin_gd_C_la_SOURCES = \
        gvplugin_gd.c \
        gvrender_gd.c \
diff --git a/plugin/gd/gdioctx_wrapper.h b/plugin/gd/gdioctx_wrapper.h
new file mode 100644 (file)
index 0000000..708cc30
--- /dev/null
@@ -0,0 +1,14 @@
+#pragma once
+
+#include <gd.h>
+#include <gvc/gvcext.h>
+#include <stddef.h>
+
+typedef struct {
+  gdIOCtx ctx;
+  GVJ_t *job;
+} gd_context_t;
+
+static inline gd_context_t *get_containing_context(gdIOCtx *ctx) {
+  return (gd_context_t *)((char *)ctx - offsetof(gd_context_t, ctx));
+}
index 94dc392cfe73f3105014b935ddf8aebfe489a850..61923ad8e3d8948bf230111ec4bc7b0a9cf86548 100644 (file)
@@ -9,23 +9,27 @@
  *************************************************************************/
 
 #include "config.h"
+#include "gdioctx_wrapper.h"
 
 #include <gvc/gvplugin_device.h>
 #include <gvc/gvio.h>
 
 #include <gd.h>
+#include <stddef.h>
 
 int gvdevice_gd_putBuf (gdIOCtx *context, const void *buffer, int len)
 {
-    return gvwrite((GVJ_t *)(context->tell), buffer, len);
+    gd_context_t *gd_context = get_containing_context(context);
+    return gvwrite(gd_context->job, buffer, len);
 }
 
 /* used by gif output */
 void gvdevice_gd_putC (gdIOCtx *context, int C)
 {
+    gd_context_t *gd_context = get_containing_context(context);
     char c = C;
 
-    gvwrite((GVJ_t *)(context->tell), &c, 1);
+    gvwrite(gd_context->job, &c, 1);
 }
 
 #ifdef HAVE_PANGOCAIRO
@@ -46,11 +50,11 @@ static void gd_format(GVJ_t * job)
     unsigned int *data = (unsigned int*)(job->imagedata);
     unsigned int width = job->width;
     unsigned int height = job->height;
-    gdIOCtx ctx = {0};
+    gd_context_t gd_context = {{0}};
 
-    ctx.putBuf = gvdevice_gd_putBuf;
-    ctx.putC = gvdevice_gd_putC;
-    ctx.tell = (void*)job;    /* hide *job here */
+    gd_context.ctx.putBuf = gvdevice_gd_putBuf;
+    gd_context.ctx.putC = gvdevice_gd_putC;
+    gd_context.job = job;
 
     im = gdImageCreateTrueColor(width, height);
     switch (job->device.id) {
@@ -92,7 +96,7 @@ static void gd_format(GVJ_t * job)
 #ifdef HAVE_GD_GIF
     case FORMAT_GIF:
        gdImageTrueColorToPalette(im, 0, 256);
-       gdImageGifCtx(im, &ctx);
+       gdImageGifCtx(im, &gd_context.ctx);
         break;
 #endif
 
@@ -108,14 +112,14 @@ static void gd_format(GVJ_t * job)
         * library documentation for more details.
         */ 
 #define JPEG_QUALITY -1
-       gdImageJpegCtx(im, &ctx, JPEG_QUALITY);
+       gdImageJpegCtx(im, &gd_context.ctx, JPEG_QUALITY);
        break;
 #endif
 
 #ifdef HAVE_GD_PNG
     case FORMAT_PNG:
         gdImageTrueColorToPalette(im, 0, 256);
-       gdImagePngCtx(im, &ctx);
+       gdImagePngCtx(im, &gd_context.ctx);
         break;
 #endif
 
@@ -135,7 +139,7 @@ static void gd_format(GVJ_t * job)
        {
            /* Use black for the foreground color for the B&W wbmp image. */
             int black = gdImageColorResolveAlpha(im, 0, 0, 0, gdAlphaOpaque);
-           gdImageWBMPCtx(im, black, &ctx);
+           gdImageWBMPCtx(im, black, &gd_context.ctx);
        }
        break;
 #endif
index 6c93fb866274bc59727bd35e5433a4c4bb44241b..82db5bba867d5b7729ff68da7e52eb0a7a4554b6 100644 (file)
@@ -9,6 +9,7 @@
  *************************************************************************/
 
 #include "config.h"
+#include "gdioctx_wrapper.h"
 
 #include <stdbool.h>
 #include <stdlib.h>
@@ -150,11 +151,11 @@ static void gdgen_end_page(GVJ_t * job)
 {
     gdImagePtr im = (gdImagePtr) job->context;
 
-    gdIOCtx ctx = {0};
+    gd_context_t gd_context = {{0}};
 
-    ctx.putBuf = gvdevice_gd_putBuf;
-    ctx.putC = gvdevice_gd_putC;
-    ctx.tell = (void*)job;    /* hide *job here */
+    gd_context.ctx.putBuf = gvdevice_gd_putBuf;
+    gd_context.ctx.putC = gvdevice_gd_putC;
+    gd_context.job = job;
 
     if (!im)
        return;
@@ -172,9 +173,9 @@ static void gdgen_end_page(GVJ_t * job)
        case FORMAT_GIF:
 #ifdef HAVE_GD_GIF
            gdImageTrueColorToPalette(im, 0, 256);
-           gdImageGifCtx(im, &ctx);
+           gdImageGifCtx(im, &gd_context.ctx);
 #else
-            (void)ctx;
+            (void)gd_context;
 #endif
            break;
        case FORMAT_JPEG:
@@ -188,13 +189,13 @@ static void gdgen_end_page(GVJ_t * job)
             * be near optimal for many applications).  See the IJG JPEG
             * library documentation for more details.  */
 #define JPEG_QUALITY -1
-           gdImageJpegCtx(im, &ctx, JPEG_QUALITY);
+           gdImageJpegCtx(im, &gd_context.ctx, JPEG_QUALITY);
 #endif
 
            break;
        case FORMAT_PNG:
 #ifdef HAVE_GD_PNG
-           gdImagePngCtx(im, &ctx);
+           gdImagePngCtx(im, &gd_context.ctx);
 #endif
            break;
 
@@ -203,7 +204,7 @@ static void gdgen_end_page(GVJ_t * job)
            {
                /* Use black for the foreground color for the B&W wbmp image. */
                int black = gdImageColorResolveAlpha(im, 0, 0, 0, gdAlphaOpaque);
-               gdImageWBMPCtx(im, black, &ctx);
+               gdImageWBMPCtx(im, black, &gd_context.ctx);
            }
            break;
 #endif