CMake: fix: use a relative 'DATA_INSTALL_DIR' on non-Linux
Some installers like NSIS on Windows do not allow absolute paths in installation
destinations. This is already followed in e.g. `BINARY_INSTALL_DIR`, but
`DATA_INSTALL_DIR` was using an absolute path. This issue was not detected
because this is currently only used in Smyrna which is not enabled on Windows in
CI. But an upcoming commit enables general example installation, which exposes
this issue.
gc: replace inline stack implementation with generic one
This code was using two abstractions, a block `blk_t` and stack `stk_t`, to
amortize the cost of allocations. We can remove the block abstraction and
rewrite the stack implementation to use the simpler generic stack while still
retaining these amortization benefits. Note that this refactoring also makes
initialization of the stack data structure unnecessary as a zeroed `gv_stack_t`
is also a valid empty stack.
The new code also deallocates the stack prior to exit, aiding tools like
Valgrind and Address Sanitizer.
This pattern of using both a hand-rolled block and hand-rolled stack appears in
numerous places in the Graphviz code base, of which this is just one instance.
Similar to prior abstractions like `bitarray_t`, this is implemented header-only
so as to be usable throughout the Graphviz tree, even by code that is not
linking against cgraph.
Given this implementation is header-only, it is natural to wonder why the type
needs a `gv_` prefix. The answer is that one of the macOS system headers flouts
the rule of `__` prefixing symbols that are part of the implementation and
defines a typedef of `__darwin_sigaltstack` under the name `stack_t`. Hence this
name is not usable by us.
macOS provides an `INT8_C` macro that generates an `int`, not an `int8_t`, which
ends up resulting in this code generating `printf` format warnings. An upcoming
commit which enables `-Werror` on this code causes test failures without this.
It seems to have been an accidental omission that these were not removed in 2d95aab626184f35f779bbd02a000a992826047a when migrating to describing link
dependencies in the build system files. However, these dependencies are typoed
too (should be `cgraph.lib` not `graph.lib`), so it is unclear how they could
have ever worked. This seems to have gone undiscovered because the Visio plugin
is not integrated into the MS Build files nor built on any Windows platform in
CI. This changes in an upcoming commit, exposing:
LINK : fatal error LNK1104: cannot open file 'graph.lib'
LASi plugin: support newer Pango weights introduced ≥1.24
Squashes a number of compiler warnings that fail the upcoming CMake build of
this plugin. The version checks and mapping logic was derived from the Pango
docs¹ and the LASi.h header.
Lasi plugin: fix: use buffered I/O instead of raw I/O
On non-Windows operating systems that lack `mmap`, this code fell back to
calling `read` but was not #including unistd.h. The result was a compilation
failure:
plugin/lasi/gvloadimage_lasi.c:78:17: error: implicit declaration of function
'read'; did you mean 'fread'? [-Werror=implicit-function-declaration]
78 | read(fd, us->data, statbuf.st_size);
| ^~~~
| fread
By moving to the higher level `fread` function we fix this problem as well as
enabling prefetching optimizations and avoiding `EINTR` complications.
This issue was discovered while attempting to enable this plugin in the CMake
build system.
Quartz plugin quartzgen_begin_page: remove open coded 'MAP_FAILED'
Later on this code goes on to do an unchecked `mmap`, so it is unclear exactly
what the author believed the failure semantics of `mmap` are. Nevertheless, this
seems like a slight improvement.
It is unclear how valuable this is, given this is only applicable to macOS and
its dependency discovery (based on the Autotools behavior) is fairly ad hoc. But
we still drag it into the CMake world to make progress on unifying the Graphviz
build system.
Poppler plugin gvloadimage_poppler_load: fix: match Glib allocation and free
The variable `absolute` is allocated using Glib’s `g_strdup` and friends.
Quoting the Glib docs:¹
It's important to match `g_malloc()` (and wrappers such as `g_new()`) with
`g_free()`, `g_slice_alloc()` (and wrappers such as `g_slice_new()`) with
`g_slice_free()`, plain `malloc()` with `free()`, and (if you're using C++)
`new` with `delete` and `new[]` with `delete[]`. Otherwise bad things can
happen, since these allocators may use different memory pools (and new/delete
call constructors and destructors).
So a custom allocation scheme or arena can be in play. Basically if you
`g_strdup` and then pair this with `free` (as was done in the code prior to this
commit), you risk leaking memory from the Glib pool and corrupting your system
allocator.
Having said that, this is no longer a concern in newer Glib:
Since GLib 2.46 `g_malloc()` is hardcoded to always use the system malloc
implementation.
03a5c3621e3185e4ca116805317a98cdc8595443 introduced another usage of the format
specifier `%zu` which has some compatibility issues on Windows. This change
switches to what it should have done to begin with, used the compatibility shim.
projects / graphviz / log