From: Matthew Fernandez Date: Thu, 3 Mar 2022 06:56:11 +0000 (-0800) Subject: GD plugin: remove repurposing of 'tell' member of I/O context X-Git-Tag: 4.0.0~177^2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f24a81e0a1c299f4557d6c125f682e28b19e8304;p=graphviz GD plugin: remove repurposing of 'tell' member of I/O context 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 --- diff --git a/plugin/gd/CMakeLists.txt b/plugin/gd/CMakeLists.txt index 284254a6a..fbe38c815 100644 --- a/plugin/gd/CMakeLists.txt +++ b/plugin/gd/CMakeLists.txt @@ -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 diff --git a/plugin/gd/Makefile.am b/plugin/gd/Makefile.am index e56fbf249..d5d7f56ce 100644 --- a/plugin/gd/Makefile.am +++ b/plugin/gd/Makefile.am @@ -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 index 000000000..708cc3062 --- /dev/null +++ b/plugin/gd/gdioctx_wrapper.h @@ -0,0 +1,14 @@ +#pragma once + +#include +#include +#include + +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)); +} diff --git a/plugin/gd/gvdevice_gd.c b/plugin/gd/gvdevice_gd.c index 94dc392cf..61923ad8e 100644 --- a/plugin/gd/gvdevice_gd.c +++ b/plugin/gd/gvdevice_gd.c @@ -9,23 +9,27 @@ *************************************************************************/ #include "config.h" +#include "gdioctx_wrapper.h" #include #include #include +#include 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 diff --git a/plugin/gd/gvrender_gd.c b/plugin/gd/gvrender_gd.c index 6c93fb866..82db5bba8 100644 --- a/plugin/gd/gvrender_gd.c +++ b/plugin/gd/gvrender_gd.c @@ -9,6 +9,7 @@ *************************************************************************/ #include "config.h" +#include "gdioctx_wrapper.h" #include #include @@ -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