GDI+ plugin: rewrite 'FileStream' reference counting to be thread safe
Based on Microsoft sample code,¹ it seems like the expectation is that consumers
of the `IStream` API may expect to concurrently manipulate reference counts.
This change guards against this by making the referencing counting code use
lock-free atomics, based on the same Microsoft sample.
Note that this is essentially proactively addressing a latent issue as it is, in
general, unsafe to use Graphviz libraries or plugins in a multithreaded
environment right now.
GDI+ plugin: use 'override' instead of 'virtual' on 'FileStream' methods
This C++11 mechanism is an improvement as it causes compilation to error out if
any of these methods were not already declared virtual in a parent class. That
is, it provides a sanity check that these overrides are really doing what they
intend.
This variable affects `find_file` and `find_path`.¹ `find_file` is not used in
Graphviz and `find_path` already defaults to the correct values for the current
platform.² This change avoids, among other things, hard-coding of the target
architecture as x86-64.
Related to #1973.
¹ see https://cmake.org/cmake/help/latest/variable/CMAKE_INCLUDE_PATH.html#variable:CMAKE_INCLUDE_PATH
² https://cmake.org/cmake/help/latest/command/find_path.html#command:find_path
The code base extensively uses `getenv` without checking the `HAVE_GETENV` macro
this detection supplies. This is not a problem because `getenv` is provided as
far back as C89, and can be guaranteed to exist on all platforms for which
Graphviz currently compiles, as we require C99.
The if-style used in CMake files had no consistency. Meanwhile, the expression
in a CMake `endif` is ignored. We learned in the process of constructing the
prior commit that this is supported right back to CMake 3.1. So this commit
standardizes on no-space, no-expression if-style.
Commit 354ed2c644452f300b316d166386bd40d8fec76f, among others, introduced the
use of `$<TARGET_OBJECTS>` in `target_link_libraries` calls. The CMake docs¹
claim this is only officially supported from 3.21 onwards, despite working in
some prior versions. CMake provides binaries going back many versions² so it is
possible to manually bisect exactly what this wording in the CMake docs means.
Running this process yields the answer that CMake 3.9 is the first version that
does not error out when encountering `$<TARGET_OBJECTS>` in a
`target_link_libraries` expression.
Note that the Graphviz CMake files also use some features like `endif` without
an expression, which claims to only be supported from CMake 3.14 onwards.³
Despite this, versions as far back as CMake 3.1 support this syntax just fine.
¹ https://cmake.org/cmake/help/latest/command/target_link_libraries.html#linking-object-libraries-via-target-objects
² https://cmake.org/files/
³ https://cmake.org/cmake/help/v3.13/command/endif.html vs
https://cmake.org/cmake/help/v3.14/command/endif.html
edgepaint: remove 'LIBRARY' declaration from the MSVC defs file
Similar to the prior commit and 2b92772d480f321bda6615140efa546abc5c66e9, this
is a compatibility change that allows MinGW to also understand this file
correctly.
Magnus Jacobsson [Sun, 28 Nov 2021 12:32:28 +0000 (13:32 +0100)]
pathplan: don't override library name in .def file
Similarly as reported in
https://gitlab.com/graphviz/graphviz/-/issues/2156, the export name
override breaks the MinGW and Cygwin autotools builds. By not
specifying the export name, each toolchain should use its appropriate
default, which should at least work, even if it's not consistent
across toolchains.
This header is not shipped and not included by anything except C sources.
Moreover these guards gave the incorrect impression this header was usable from
C++. One of its structs contains a field, `final`, that is a C++ keyword, so it
is not possible to use this code from C++.
smyrna pick_object: replace use of large constant with 'FLT_MAX'
The compiler warns:
selectionfuncs.c: In function 'pick_object':
selectionfuncs.c:114:16: warning: conversion from 'int' to 'float' changes
value from '999999999' to '1.0e+9f' [-Wfloat-conversion]
114 | float dist=999999999;
| ^~~~~~~~~
The usage of `dist` in this function is really that its initial value needs to
be greater than any other value encountered, so `FLT_MAX` serves this purpose
better.
Commit 0b31ec29078ecee5b868b42453478f1dd8239d54 modified `checksum` so it only
produced a single checksum, SHA256. Therefore we can simplify the function to
just return that single checksum file instead of defining it as a generator.
This allows the script itself to control text encoding used in the output. A
future commit will use this to avoid awkward juggling of locales and encoding
externally.
gdiplus plugin: make 'FileStream' destructor virtual
`FileStream::Release` optionally deletes `this`. This is OK because nothing
inherits from `FileStream`, but the compiler believes this is dangerous because
this class participates in an inheritance hierarchy and does not have a virtual
destructor. By giving it one, we silence the -Wdelete-non-virtual-dtor warning,
make it safer to inherit from this class in future, and make it safe to delete
a `FileStream` through an `IStream` pointer.
cgraph: go through 'uintptr_t' when casting between pointers as IDs
The type `IDTYPE` is `uint64_t` which is wide enough to contain a pointer on all
supported platforms. However, on 32-bit platforms the compiler throws
-Wpointer-to-int-cast about this kind of back-and-forth shuffling. Going through
`uintptr_t` (or `intptr_t`) silences these.
projects / graphviz / log