This change has no effect on functionality, but strchr is cheaper to call and
equivalent to these strstr calls. This likely makes no difference in an
optimized build as modern compilers can see this transformation is possible
themselves. However, this change may assist older compilers or accelerate
unoptimized builds.
fix: remove hard limit of 1000 boxes in dot spline code
The dot spline code used a static array of 1000 box data structures. It is not
clear to me why this limit was thought to be enough. I suspect the limit (added
in the initial Graphviz revision) was just thought to be something sufficiently
large that no user would ever hit it.
This is no longer true. The graph in issue #2095 exceeds this limit. Because
there are no bounds checks when moving through the boxes array, this resulted in
a crash due to a write beyond the end of the array.
In this commit, we remove this limit entirely and instead use a dynamically
allocated expanding array of boxes. This permits handling an arbitrary number of
boxes during computation. Fixes #2095.
make_flat_edge: use a local box array instead of the global boxes
The values written to this array during make_flat_edge do not need to be
retained after the function returns. It was simply using the boxes as scratch
space. Using a local array instead makes it easier for the compiler to optimize
and will ease some upcoming changes. Related to #2095.
This introduces a new -Wshadow compiler warning, but that will be removed in a
future commit.
make_flat_bottom_edges: use a local box array instead of the global boxes
The values written to this array during make_flat_bottom_edges do not need to be
retained after the function returns. It was simply using the boxes as scratch
space. Using a local array instead makes it easier for the compiler to optimize
and will ease some upcoming changes. Related to #2095.
This introduces a new -Wshadow compiler warning, but that will be removed in a
future commit.
make_flat_labeled_edge: use a local box array instead of the global boxes
The values written to this array during make_flat_labeled_edge do not need to be
retained after the function returns. It was simply using the boxes as scratch
space. Using a local array instead makes it easier for the compiler to optimize
and will ease some upcoming changes. Related to #2095.
This introduces a new -Wshadow compiler warning, but that will be removed in a
future commit.
As I look into how to resolve this issue, I realize it does not make sense to
look for mention of a subgraph because the layout attribute is invalid on *all*
entities except graphs. That is, the error/warning should just say it is only
valid for graphs. Related to #2078.
rewrite streq as a function and remove micro-optimization
There is no need for this to be a macro or for it to check the first character
explicitly. Modern compilers can do this kind of optimization themselves.
I do not know why the comment says it is surprising the assertion fails. It
seems perfectly normal to me that it would fail, as the correct condition is
against *both* AGINEDGE and AGOUTEDGE as in the following line.
There is no need for this to be a macro. Making it a function allows stronger
type safety, safe to use with impure arguments, and reduces bracketing noise.
agrefstrdump: dump dictionary associated with the current graph, not default
It looks like a mistake that this function was ignoring its input argument and
always dumping the default string dictionary. This change makes the function
dump the string dictionary associated with the passed graph. This function is
only compiled in when DEBUG is defined and it is not exposed to users, so no
changelog entry for this. Fixes #1985.
SparseMatrix_copy: avoid calling memcpy with null pointers
The memcpy function technically requires both its pointer inputs to be non-null.
There is no special case for the length being 0. When running the example from
#2088 under UBSan, it detects this memcpy call as being performed with both null
source and destination pointers. It is unlikely any real world memcpy would
misbehave in this circumstance, but it is still good practice to avoid this.
disable -Wunused-parameter when building SWIG-generated Guile bindings
There is no value to warning about things like this in generated code, which are
just artifacts of how SWIG code generation works. Removes 68 compiler warnings.
deal exclusively in size_t in Java bindings support code
In this code, committed in c7374588519a7ac557c14b38c147623dedf43fdb in 2014, it
is not clear to me why int was used. The correct type for these kind of
operations is size_t. The size_t type has been available since at least 1995, if
not earlier depending on your toolchain. Squashes 3
-Wsign-conversion/-Wconversion warnings.
disable -Wunused-variable when building SWIG-generated OCaml bindings
There is no value to warning about things like this in generated code, which are
just artifacts of how SWIG code generation works. Removes 112 compiler warnings.
disable -Wunused-function when building SWIG-generated OCaml bindings
There is no value to warning about things like this in generated code, which are
just artifacts of how SWIG code generation works. Removes 24 compiler warnings.
disable -Wunused-label when building SWIG-generated PHP bindings
There is no value to warning about things like this in generated code, which are
just artifacts of how SWIG code generation works. Removes 95 compiler warnings.
disable -Wunused-parameter when building SWIG-generated PHP bindings
There is no value to warning about things like this in generated code, which are
just artifacts of how SWIG code generation works. Removes 4 compiler warnings.
gv_string_writer: gracefully handle sizes that exceed INT_MAX
Similar to the previous commit, this deals with some mismatch between the TCL
API expecting int and the Graphviz API dealing in size_t.
This stems from a mismatch between gv_channel_write (or back tracking this, the
write_fn member of GVC) dealing in size_t and the TCL API dealing in int. Prior
to this change, a write of more than INT_MAX through this function would result
in passing a negative value to TCL. I do not know how it responds to such
things. Following this change, a write of more than INT_MAX results in a
truncated write of at most INT_MAX, something callers should already be
anticipating. Squashes two compiler warnings.
gv_channel_writer: gracefully handle sizes that exceed INT_MAX
The compiler (correctly) says:
gv_tcl_init.c: In function ‘gv_channel_writer’:
gv_tcl_init.c:25:58: warning: conversion from ‘size_t’ {aka ‘long
unsigned int’} to ‘int’ may change value [-Wconversion]
return Tcl_Write((Tcl_Channel)(job->output_file), s, len);
^~~
gv_tcl_init.c:25:12: warning: conversion to ‘size_t’ {aka ‘long unsigned
int’} from ‘int’ may change the sign of the result [-Wsign-conversion]
return Tcl_Write((Tcl_Channel)(job->output_file), s, len);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This stems from a mismatch between gv_channel_write (or back tracking this, the
write_fn member of GVC) dealing in size_t and the TCL API dealing in int. Prior
to this change, a write of more than INT_MAX through this function would result
in passing a negative value to TCL. I do not know how it responds to such
things. Following this change, a write of more than INT_MAX results in a
truncated write of at most INT_MAX, something callers should already be
anticipating. Squashes two compiler warnings.
disable -Wunused-parameter when building SWIG-generated Ruby bindings
There is no value to warning about things like this in generated code, which are
just artifacts of how SWIG code generation works. Removes 91 compiler warnings.
remove unnecessary not-null guards to free in Operator_diag_precon_delete
It is well-defined to free null. Apart from this though, the first condition
containing a dereference of `o` implies that `o` is not null so the second
condition can be assumed true.
remove unnecessary branch in gvplugin_library_load
The function grealloc can handle the input pointer being NULL and the follow on
code after this does not rely on the contents of the allocated buffer being
zeroed.
Magnus Jacobsson [Mon, 12 Jul 2021 14:44:59 +0000 (16:44 +0200)]
add MSBuild project reference to gvc for neatogen
This is step 1 of 4 in replacing ortho with gvc as a link dependency
for neatogen. It ensures the correct build order under all
circumstances, also when neatogen is built stand-alone. Just a link
dependency is not enough which does not cause the dependency to
actually be built.
A project reference is a more modern way of creating a project
dependency than the centralized project dependencies in the solution
file. See https://stackoverflow.com/a/2317017/3122101.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
Magnus Jacobsson [Mon, 12 Jul 2021 14:26:45 +0000 (16:26 +0200)]
add MSBuild project reference to gvc for dotgen
This is step 1 of 4 in replacing ortho with gvc as a link dependency
for dotgen. It ensures the correct build order under all
circumstances, also when dotgen is built stand-alone. Just a link
dependency is not enough which does not cause the link dependency to
actually be built.
A project reference is a more modern way of creating a project
dependency than the centralized project dependencies in the solution
file. See https://stackoverflow.com/a/2317017/3122101.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
force all ortho object files to be linked into gvc for CMake and MSBuild
This is what the autotools builds do.
Whithout this, the ortho object files ended up both in
libgvplugin_dot_layout.so and libgvplugin_neato_layout.so which caused
an ASan odr_violation.
This commit also adds MSVC an import/export declaration for orthoEdges
which otherwise would become unresolved.
It also changes gvc to use PRIVATE instead of PUBLIC linking to
ortho. The reason is detailed below.
The ASan error message was:
==235826==ERROR: AddressSanitizer: odr-violation (0x7ff55b6c72c0):
[1] size=4 'odb_flags' /home/magjac/graphviz/lib/ortho/ortho.c:44:5
[2] size=4 'odb_flags' /home/magjac/graphviz/lib/ortho/ortho.c:44:5
These globals were registered at these points:
[1]:
#0 0x7ff55b8c5938 in __asan_register_globals ../../../../src/libsanitizer/asan/asan_globals.cpp:341
#1 0x7ff55b2ce1a2 in _sub_I_00099_3 (/home/magjac/graphviz/build/plugin/dot_layout/libgvplugin_dot_layout.so.6+0x43b1a2)
#2 0x7ff55c28c96d (/lib64/ld-linux-x86-64.so.2+0x1196d)
[2]:
#0 0x7ff55b8c5938 in __asan_register_globals ../../../../src/libsanitizer/asan/asan_globals.cpp:341
#1 0x7ff55a8008e4 in _sub_I_00099_3 (/home/magjac/graphviz/build/plugin/neato_layout/libgvplugin_neato_layout.so.6+0x6d38e4)
#2 0x7ff55c28c96d (/lib64/ld-linux-x86-64.so.2+0x1196d)
==235826==HINT: if you don't care about these errors you may set ASAN_OPTIONS=detect_odr_violation=0
SUMMARY: AddressSanitizer: odr-violation: global 'odb_flags' at /home/magjac/graphviz/lib/ortho/ortho.c:44:5
==235826==ABORTING
Here is a more detailed explanation of the problem and the fix:
1. The ortho library is not a library that we ship.
2. The ortho library is a static library.
3. The ortho object modules should be part of the gvc library. With
autotools they are, but before this fix they are not with CMake or
MSBuild.
4. A user program should load the ortho object modules from the gvc
libray. This works with autotools, but before this fix not with
CMake or MSBuild.
5. All plugins (and all other Graphviz programs and libraries) should
load the ortho object modules from the gvc library. This works with
autotools, but before this fix not with CMake or MSBuild.
6. Nothing should ever link directly to the ortho library except gvc.
7. Even before this fix, the gvc library had ortho as a PUBLIC
dependency, but since gvc itself doesn't refer to any ortho
symbols, no object modules from ortho gets included in the gvc
library itself unless we explicitly include them. Which is what the
autotools builds do and what the CMake and BSBuild builds do after
this fix. My interpretation is that without this, gvc acted as an
INTERFACE library for ortho, i.e., it could tell anything linking
to it where to find the object modules, but not provide them itself
(again, my interpretation. For more see
https://cmake.org/cmake/help/latest/command/add_library.html#interface-libraries).
8. The dot_layout and neato_layout plugins have the gvc library as a
public dependency, but they don't have any direct dependency on the
ortho library itself. Because of this, they find the ortho objects
modules that they need though the gvc library. Before this fix
however, since gvc did not contain the ortho objects, they had no
option but to link in the ortho object modules statically. This is
what caused the ODR violation when both plugins were loaded
simultaneously.
9. This fix also changes the gvc dependency on ortho from PUBLIC to
PRIVATE to avoid giving anything access to the ortho static library
through gvc.
Magnus Jacobsson [Mon, 12 Jul 2021 13:42:05 +0000 (15:42 +0200)]
add MSBuild project reference to ortho for gvc
This is a preparation for adding ortho as a link dependency to gvc in
an upcoming commit. It ensures the correct build order under all
circumstances, also when gvc is built stand-alone. A link dependency
is not enough which does not cause the link dependency to actually be
built.
A project reference is a more modern way of creating a project
dependency than the centralized project dependencies in the solution
file. See https://stackoverflow.com/a/2317017/3122101.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
Magnus Jacobsson [Mon, 12 Jul 2021 05:43:34 +0000 (07:43 +0200)]
remove useless MSBuild link dependency on ortho for gvmap
gvmap only depends indirectly on ortho through neatogen which it
already has a link dependency on.
This change is made to avoid confusion when upcoming commits in this
series change ortho to be part of the gvc shared library and adjusts
the neatogen library accordingly.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
Magnus Jacobsson [Mon, 12 Jul 2021 14:56:13 +0000 (16:56 +0200)]
remove useless MSBuild project dependency on ortho for gvmap
gvmap only depends indirectly on ortho through neatogen which it
already has a project dependency on.
This change is made to avoid confusion when upcoming commits in this
series change ortho to be part of the gvc shared library and adjusts
the neatogen library accordingly.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
Magnus Jacobsson [Mon, 12 Jul 2021 05:37:19 +0000 (07:37 +0200)]
remove useless MSBuild project reference to ortho for dot
dot only depends indirectly on ortho through dotgen which it already
has a project reference to.
This change is made to avoid confusion when upcoming commits in this
series change ortho to be part of the gvc shared library and adjusts
the dotgen library accordingly.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
Magnus Jacobsson [Mon, 12 Jul 2021 14:54:05 +0000 (16:54 +0200)]
remove useless MSBuild project dependency on ortho for neato_layout plugin
The neato_layout plugin only depends indirectly on ortho through
neatogen which it already has a project reference to.
A project reference is a more modern way of creating a project
dependency than the centralized project dependencies in the solution
file. See https://stackoverflow.com/a/2317017/3122101.
This change is made to avoid confusion when upcoming commits in this
series change ortho to be part of the gvc shared library and adjusts
the neatogen library accordingly.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
Magnus Jacobsson [Mon, 12 Jul 2021 05:50:59 +0000 (07:50 +0200)]
remove useless MSBuild link dependency on ortho for neato_layout plugin
The neato_layout plugin only depends indirectly on ortho through
neatogen which it already has a link dependency on.
This change is made to avoid confusion when upcoming commits in this
series change ortho to be part of the gvc shared library and adjusts
the neatogen library accordingly.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
Magnus Jacobsson [Mon, 12 Jul 2021 05:49:57 +0000 (07:49 +0200)]
remove useless MSBuild project reference to ortho for dot_layout plugin
The dot_layout plugin only depends indirectly on ortho through dotgen
which it already has a project reference to.
This change is made to avoid confusion when upcoming commits in this
series change ortho to be part of the gvc shared library and adjusts
the dotgen library accordingly.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
Magnus Jacobsson [Mon, 12 Jul 2021 03:17:43 +0000 (05:17 +0200)]
remove useless MSBuild link dependency on ortho for dot_layout plugin
The dot_layout plugin only depends indirectly on ortho through dotgen
which it already has a link dependency on.
This change is made to avoid confusion when upcoming commits in this
series change ortho to be part of the gvc shared library and adjusts
the dotgen library accordingly.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
remove unnecessary './' prefix on report.txt output in CI
Commit 8f7c1f4c2ef253bc1a763515b344d599e5c199ee removed any directory prefix
from this artifact, outputting it to the root directory of the Graphviz checkout
in CI. In this case, like metrics.txt, it does not need an explicit './' prefix.
gxl2gv: fix: recognize and handle HTML-like strings
When translating a Graphviz file through gv2gxl, HTML-like and non-HTML-like
strings would be translated identically, resulting in a loss of information.
This was fixed in the series merged in d44d19c86f0f469d1c76ce0d7630c8e0ac85e993.
However, to take this through to completion and make round-tripping such strings
possible, changes to gxl2gv were also required.
The present commit updates gxl2gv to recognize the new ‘kind’ GXL attribute that
gv2gxl uses to indicate a string originated as an HTML-like string. The output
of gxl2gv now correctly translates this back to an HTML-like string. Fixes #517.
Note that this fix is orthogonal to the behavior when using an HTML-like and
non-HTML-like string with the same content. It is still not possible to do this
without confusing the internal string interning dictionary (#2089).
gxl2gv: use an enum instead of #defines for attribute type
This introduces some stronger typing and makes it clearer that the attribute
type variables like Current_class can only ever be one of these values. Related
to #517.
An upcoming change makes the type of Current_class an enum, which introduces new
-Wswitch compiler warnings. To avoid this in advance, we give this switch a
default case that does nothing.
gxl2gv: push the ability to indicate an attribute is HTML-like outwards
Callers of setAttr can now indicate the attribute they are setting is an
HTML-like string. This is not yet taken advantage of, but this change is a step
towards #517.
projects / graphviz / log