This function was retaining a pointer to memory pointed to by its parameter,
`name`, in the global `suffix`. So it is unsafe to call this with anything
except a long lived string. We can make this more obvious/explicit by inlining
the use of `outfile`, the only parameter it is ever called with.
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.
The GD plugin was creating `gdIOCtx` objects on the stack with some
uninitialized members. At time of writing, the GD docs¹ claim this struct’s
layout is:
typedef struct gdIOCtx
{
int (*getC)(gdIOCtxPtr);
int (*getBuf)(gdIOCtxPtr, void *, int wanted);
void (*putC)(gdIOCtxPtr, int);
int (*putBuf)(gdIOCtxPtr, const void *, int wanted);
// seek must return 1 on SUCCESS, 0 on FAILURE. Unlike fseek!
int (*seek)(gdIOCtxPtr, const int);
long (*tell)(gdIOCtxPtr);
void (*gd_free)(gdIOCtxPtr);
} gdIOCtx;
So Graphviz’ usage was leaving `getC`, `getBuf`, `seek`, and `gd_free`
uninitialized. This seems to work out OK; Graphviz’ usage of libgd apparently
does not involve any code paths that use these members. But this does not seem
to be an API guarantee. This change zeroes these members for future stability.
gvplugin_install: remove 63-character limit on plugin names
This change rephrases `gvplugin_install` to avoid the use of intermediate
buffers and, in the process, removes a constraint on the maximum length of
plugin names. That is, this is a performance improvement and a functional
improvement.
avoid use of '__builtin_unreachable' in 'UNREACHABLE' macro
There was some discussion on !2503¹ around the effect of `__builtin_unreachable`
and whether we are really confident applying such a strong compiler hint to
complex control flow logic that is, in some cases, only partially understood.
This change softens the effect of the `UNREACHABLE` macro to a reliable program
abort rather than undefined behavior if mistakenly tagged unreachable code is
reached. The result is slightly less efficient but safer code.
fix corruption of user shape characteristics during EPSF initialization
From the surrounding context, it is clear this code was intending to set `h`,
the height of the shape, not overwrite the `y` coordinate it had previously set
for the shape. This change not only fixes the overwrite of `us->y` but fixes a
read of uninitialized memory in `us->h` by the caller of this function.
It is not clear to me what the full user-visible effect of this change is.
Costa Shulyupin [Thu, 10 Mar 2022 07:47:14 +0000 (09:47 +0200)]
fix out of source build error in lib/expr
make[3]: Entering directory '/home/costa/oss/graphviz/build/lib/expr'
CC excc.lo
In file included from ../../../lib/expr/expr.h:28,
from ../../../lib/expr/exlib.h:126,
from ../../../lib/expr/excc.c:25:
../../../lib/expr/exparse.h:4:10: fatal error: expr/y.tab.h: No such file or directory
4 | #include <expr/y.tab.h>
| ^~~~~~~~~~~~~~
compilation terminated.
Costa Shulyupin [Thu, 10 Mar 2022 07:40:06 +0000 (09:40 +0200)]
fix out of source build error in lib/common
make[4]: Entering directory '/home/costa/oss/graphviz/build/lib/common'
CC arrows.lo
../../../lib/common/arrows.c: In function ‘arrowOrthoClip’:
.....
../../../lib/common/colxlate.c:21:10: fatal error: common/colortbl.h: No such file or directory
21 | #include <common/colortbl.h>
| ^~~~~~~~~~~~~~~~~~~
compilation terminated.
make[3]: Entering directory '/home/costa/oss/graphviz/build/lib/gvpr'
CC actions.lo
In file included from ../../../lib/expr/expr.h:28,
from ../../../lib/gvpr/actions.h:18,
from ../../../lib/gvpr/actions.c:16:
../../../lib/expr/exparse.h:4:10: fatal error: expr/y.tab.h: No such file or directory
4 | #include <expr/y.tab.h>
| ^~~~~~~~~~~~~~
compilation terminated.
core plugin map_output_shape: replace dynamic buffer with inline conversion
This function was using a long lived buffer to hold conversions of the input
array `AF`. However each entry was only used once or twice. It is faster to
avoid the heap buffer altogether and simply do the conversions on-demand when
they are needed.
This change is expected to be a slight performance improvement as well as
further progress towards thread safety in this code.
core plugin: remove assumptions that pen width fits in an int
By dealing with this field consistently as a double, we retain the previous
intent while removing assumptions on the characteristics of `int`. Squashes 8
compiler warnings.
GNU Make supports both this syntax and `patsubst` for pattern substitution. The
former is more portable to other Make implementations, so use this in preference
to the latter.
This file has a fairly strange structure where almost every use of `cur_level`
is actually referencing a local that shadows the global `cur_level`. The global
`cur_level` is never written to and remains 0 throughout execution, making it
somewhat useless.
Criterion is a testing framework that has been used for writing Graphviz unit
tests in the past (see tests/unit_tests). These unit tests are not currently
enabled. However, Windows CI unconditionally checks out all submodules, thus
pulling in a version of Criterion that goes unused.
This change removes the previously described behavior. If/when the
Criterion-based unit tests are resurrected, an up to date version of Criterion
can be pulled in via a package manager or the Windows build dependencies
repository¹ instead.
This change should slightly accelerate Windows CI jobs.
GD plugin: use a dynamically allocated VRML rendering state instead of globals
This brings the affected code closer to being thread safe. The goal is to allow
(1) multiple threads to execute in this code safely as well as (2) multiple VRML
renderings on the same thread to be interleaved with each other.
GD plugin: remove unnecessary common/utils.h include
This was introduced in ff97490f4de09db950f708b1bf44a8f8f62902c5 in the same
commit that _removed_ all usage of `late_double` here, so it is unclear what the
intended meaning of the leading comment was.
1. Everything, with warning count piped to metrics.txt
2. Everything except the check for FIXMEs, hard failing if there is any
warning
The latter passes. That is, all Graphviz Python code is warning-free with the
exception of FIXMEs. Meanwhile FIXMEs are frequently _intentionally_ introduced
in test code, indicating a known open bug for which a test case is being added.
So metric collection in step 1 above is not relevant; we expect this number
often to go up. This commit removes the collection of this unused metric to
decrease CI complexity.
The return value of `execvp` is irrelevant because this function only ever
returns when it fails. In this situation, the failure cause is in the `errno`
global, not in the return value.
gvc: [nfc] remove parens around return value style
This is an idiom occasionally used to allow defining `return` as a macro that
does some additional instrumentation. This style has mostly fallen out of favor,
with mechanisms like `-finstrument-functions` instead used as a more robust way
of achieving the same thing.
The build system does not check for `g_type_term` so this code is always
disabled. More importantly though, I cannot locate any reference to a
`g_type_term` function that has ever existed anywhere except the Graphviz code
base. That is, it appears this has never been a librsvg or Glib function. The
commit message of its introduction in 9f4cb1fcb745b9b71c937980ef6b0d08552ead0f
also offers no enlightenment, only suggesting this was somehow related to Fedora
20.
DevIL plugin: squash -Wsign-conversion warning for 'ilSaveF' call
The DevIL plugin provides a number of output devices for which it claims IDs
matching the DevIL `ILenum` types. That is, it effectively stashes the DevIL
output format in the Graphviz device ID. This change simply makes it more
obvious to the compiler that the translation back and forth is intentional. For
more information, see the DevIL manual.¹
According to the DevIL API docs,¹ any error from `ilTexImage` is reported via
`ilGetError`, not through its return value. Squashes a -Wunused-but-set-variable
warning.
Pango plugin: fix: do not judge empty lines as failing during text layout
Text layout plugins are expected to return failure as `false` from their
`textlayout` function. The Pango plugin considered failure to be anything that
resulted in a horizontal layout width of 0. However, this is not a failure in
the case where the text being laid out is the empty string; its horizontal width
is expected to be 0.
The effect of this was that HTML-like strings like `<<br/>1>` were judged to
fail during text layout and a (redundant) estimation of their text width was
performed. This seems to have been a latent bug present since commit ad82ef8613b0731806b81f9c0047d0cbf6745470 (~July 2007). This recently became more
visible due to commit 7aa0dcc03ea20b544b2463d97fe4a78af699589c that introduced
warnings during text width estimation if a fallback metric needed to be used.
Users were now presented with “Warning: no hard-coded metrics” when using fonts
that Pango knew of and should not have needed estimation in the first place.
This fix makes the Pango plugin consider 0 width for the layout of an empty
string to be successful. To be clear, this commit is both a functional fix and a
performance improvement.
CMake: enable libgd support on macOS when possible
As discussed in the comment in this commit, we hard code the options the
Homebrew libgd package is built with.¹ It seems not possible to build a libgd
without GIF support, so perhaps the `HAVE_GD_GIF` logic in the Graphviz code
base could be simplified in future.
projects / graphviz / log