The lib/cgraph/alloc.h wrappers are similar to the older lib/common/memory.h
wrappers except (1) they are header-only and (2) they live in a directory
(cgraph) that is at the root of the dependency tree. The long term plan is to
replace all use of lib/common/memory.h with lib/cgraph/alloc.h.
fdpgen expandCluster: use cgraph wrapper for allocation
The lib/cgraph/alloc.h wrappers are similar to the older lib/common/memory.h
wrappers except (1) they are header-only and (2) they live in a directory
(cgraph) that is at the root of the dependency tree. The long term plan is to
replace all use of lib/common/memory.h with lib/cgraph/alloc.h.
fdpgen getEdgeList: use cgraph wrapper for allocation
The lib/cgraph/alloc.h wrappers are similar to the older lib/common/memory.h
wrappers except (1) they are header-only and (2) they live in a directory
(cgraph) that is at the root of the dependency tree. The long term plan is to
replace all use of lib/common/memory.h with lib/cgraph/alloc.h.
fdpgen deriveGraph: use cgraph wrapper for allocation
The lib/cgraph/alloc.h wrappers are similar to the older lib/common/memory.h
wrappers except (1) they are header-only and (2) they live in a directory
(cgraph) that is at the root of the dependency tree. The long term plan is to
replace all use of lib/common/memory.h with lib/cgraph/alloc.h.
The lib/cgraph/alloc.h wrappers are similar to the older lib/common/memory.h
wrappers except (1) they are header-only and (2) they live in a directory
(cgraph) that is at the root of the dependency tree. The long term plan is to
replace all use of lib/common/memory.h with lib/cgraph/alloc.h.
fdpgen addCluster: use cgraph wrapper for allocation
The lib/cgraph/alloc.h wrappers are similar to the older lib/common/memory.h
wrappers except (1) they are header-only and (2) they live in a directory
(cgraph) that is at the root of the dependency tree. The long term plan is to
replace all use of lib/common/memory.h with lib/cgraph/alloc.h.
fdpgen mkDeriveNode: use cgraph wrappers for allocation
The lib/cgraph/alloc.h wrappers are similar to the older lib/common/memory.h
wrappers except (1) they are header-only and (2) they live in a directory
(cgraph) that is at the root of the dependency tree. The long term plan is to
replace all use of lib/common/memory.h with lib/cgraph/alloc.h.
As far as I can tell, this code has never been compilable. It relies on `CellW`
and `CellH`, neither of which have ever been defined in Graphviz’ history.
The majority of Graphviz code was using the types `intptr_t` and `uintptr_t`
unconditionally. So it seems safe to assume that any compliant C99 environment
has both of these.
The Intel C Compiler is now based on LLVM. We think this means it has a Clang
compatible front end, though we do not have access to it to check. If this is
accurate, it means the build system no longer needs any specialized ICC support.
neatogen makeMultiSpline: use cgraph wrapper for allocation
The lib/cgraph/alloc.h wrappers are similar to the older lib/common/memory.h
wrappers except (1) they are header-only and (2) they live in a directory
(cgraph) that is at the root of the dependency tree. The long term plan is to
replace all use of lib/common/memory.h with lib/cgraph/alloc.h.
neatogen triPath: use cgraph wrapper for allocation
The lib/cgraph/alloc.h wrappers are similar to the older lib/common/memory.h
wrappers except (1) they are header-only and (2) they live in a directory
(cgraph) that is at the root of the dependency tree. The long term plan is to
replace all use of lib/common/memory.h with lib/cgraph/alloc.h.
neatogen mkPoly: use cgraph wrapper for allocation
The lib/cgraph/alloc.h wrappers are similar to the older lib/common/memory.h
wrappers except (1) they are header-only and (2) they live in a directory
(cgraph) that is at the root of the dependency tree. The long term plan is to
replace all use of lib/common/memory.h with lib/cgraph/alloc.h.
neatogen genroute: use cgraph wrapper for allocation
The lib/cgraph/alloc.h wrappers are similar to the older lib/common/memory.h
wrappers except (1) they are header-only and (2) they live in a directory
(cgraph) that is at the root of the dependency tree. The long term plan is to
replace all use of lib/common/memory.h with lib/cgraph/alloc.h.
neatogen finishEdge: use cgraph wrapper for allocation
The lib/cgraph/alloc.h wrappers are similar to the older lib/common/memory.h
wrappers except (1) they are header-only and (2) they live in a directory
(cgraph) that is at the root of the dependency tree. The long term plan is to
replace all use of lib/common/memory.h with lib/cgraph/alloc.h.
neatogen mkRouter: use cgraph wrapper for allocation
The lib/cgraph/alloc.h wrappers are similar to the older lib/common/memory.h
wrappers except (1) they are header-only and (2) they live in a directory
(cgraph) that is at the root of the dependency tree. The long term plan is to
replace all use of lib/common/memory.h with lib/cgraph/alloc.h.
neatogen mkTriGraph: use cgraph wrapper for allocation
The lib/cgraph/alloc.h wrappers are similar to the older lib/common/memory.h
wrappers except (1) they are header-only and (2) they live in a directory
(cgraph) that is at the root of the dependency tree. The long term plan is to
replace all use of lib/common/memory.h with lib/cgraph/alloc.h.
Note that in this change we can drop some explicit initialization because the
cgraph wrappers zero-initialize.
neatogen: remove upfront allocation of trigraph edges
Following on from fc465488e1e62fe5ef879b379a07ef272431f400, this removes
assumptions about how many total edges will be generated by this algorithm. New
edges are now allocated on demand when appending.
neatogen mkTriIndices: use cgraph wrapper for allocation
The lib/cgraph/alloc.h wrappers are similar to the older lib/common/memory.h
wrappers except (1) they are header-only and (2) they live in a directory
(cgraph) that is at the root of the dependency tree. The long term plan is to
replace all use of lib/common/memory.h with lib/cgraph/alloc.h.
neatogen mkCtrlPts: use cgraph wrapper for allocation
The lib/cgraph/alloc.h wrappers are similar to the older lib/common/memory.h
wrappers except (1) they are header-only and (2) they live in a directory
(cgraph) that is at the root of the dependency tree. The long term plan is to
replace all use of lib/common/memory.h with lib/cgraph/alloc.h.
neatogen addTri: use cgraph wrapper for allocation
The lib/cgraph/alloc.h wrappers are similar to the older lib/common/memory.h
wrappers except (1) they are header-only and (2) they live in a directory
(cgraph) that is at the root of the dependency tree. The long term plan is to
replace all use of lib/common/memory.h with lib/cgraph/alloc.h.
neatogen newIpair: use cgraph wrapper for allocation
The lib/cgraph/alloc.h wrappers are similar to the older lib/common/memory.h
wrappers except (1) they are header-only and (2) they live in a directory
(cgraph) that is at the root of the dependency tree. The long term plan is to
replace all use of lib/common/memory.h with lib/cgraph/alloc.h.
neatogen newItem: use cgraph wrapper for allocation
The lib/cgraph/alloc.h wrappers are similar to the older lib/common/memory.h
wrappers except (1) they are header-only and (2) they live in a directory
(cgraph) that is at the root of the dependency tree. The long term plan is to
replace all use of lib/common/memory.h with lib/cgraph/alloc.h.
CMake: fix: teach 'gvpack' how to find plugins at run time
Because `gvpack` links against the plugins as libraries rather than loading them
on demand, the dynamic linker needs to be able to locate the plugin libraries at
run time.
CC: Jaroslav Škarvada <jskarvad@redhat.com>
Gitlab: fixes #1838
Red Hat: fixes https://bugzilla.redhat.com/show_bug.cgi?id=1838679
core plugin: stop emitting an empty 'alt' tag in cmap output
This was working around a behavior on an old version of Microsoft Internet
Explorer that is no longer supported. Tooltips should show consistently on
modern browsers and the `alt` tag is once again available to be set dynamically
by Javascript or inherited.
CI: add Qt generated sources to files excluded from coverage analysis
These will be generated by an upcoming commit adding GVEdit to the CMake build
system. The cryptic EWIEGA46WW directory in this path is a checksum generated by
`AUTOMOC`. But it is based on relative path and appears to be stable as long as
we do not rename any cmd/gvedit subdirectories.
Qt 5.15 deprecated the version of this function that returns a pointer and
throws compiler warnings when calls to it are seen. This modernization is
necessary to avoid the upcoming integration of Gvedit into the CMake build
system failing in CI where `-Werror` is applied.
Unfortunately we need to do this conditionally while retaining the old code for,
e.g., CentOS 7 which has a version of Qt 5.15 that does not have the new API.
gvedit: locate attrs.txt based on the location of our own executable
This commit adapts 24bd92c1e5d49b354141cd06d88ca658991b9825 that taught Smyrna
how to find the share directory without a build time define to Gvedit. The
pattern is essentially the same, but we can take advantage of some nicer C++
mechanisms instead of C.
The motivation for this is to make integration of Gvedit into the CMake build
system easier. By doing location of the share directory in the code instead of
the build system, we can more easily ensure the same behavior between build
systems. A side effect of this is the `gvedit` binary becoming more relocatable.
Note that the Windows discovery mechanism differs from what it was before, but
it is now more in line with how discovery works on other platforms.
neatogen: fix miscalculation of intermediate edge resources
`genroute` was allocating an array for edge computation upfront. But what it was
not accounting for was that some of the functions it later calls _change_ the
`pn` value it used to determine how many array elements it should allocate.
Specifically, `Pshortestpath` can add new points to the polygon, thereby causing
the walk of the (now too short) array to write out of bounds.
neatogen: fix out of bounds write when exceeding estimated edges
The `edgei` allocation in `mkTriGraph` was wrong. This code was written prior to
Git history, and it is not obvious how the calculation for how many edges to
allocate was arrived at. My educated guess is that the `+ 2` was intended to
account for the maximum number of final appended edges (the maximum trip count
of the trailing loop in this function), except that is 3 not 2. Bumping this to
3 indeed bypasses the ASan failure in #42.
Rather than just make that equally error prone adjustment, this commit stops
estimating the edge count upfront at all and instead allocates edges per-node
on-demand.
The prior code was allocating edges for all nodes as a contiguous array. Each
node would then be given an offset pointer into this array. Apart from the
problem described above, this meant all nodes apart from the last one could
overflow their edge count, silently corrupting their neighbor’s edges, in a way
undetectable by ASan. This refactoring gives each node distinct memory for its
edges. But this means we needed to introduce a node count (`nnodes`) to the
trigraph and free each of these separate allocations when destructing the
trigraph.
This also required passing in original edge counts to `resetGraph`. Previously
the code would detect an edge that needed to be removed by a node’s edges
running up against its neighbor’s. Obviously this was subject to the above
described bugs which could cause false negatives in this test, leading to even
further compounding data corruption.
Fixing this unfortunately only yields yet another ASan crash (below). This will
be addressed in an upcoming commit.
WRITE of size 16 at 0x613000001860 thread T0
#0 0x7f5b44579e80 in genroute lib/neatogen/multispline.c:869
#1 0x7f5b4457f53d in makeMultiSpline lib/neatogen/multispline.c:1311
#2 0x7f5b4452ac4d in _spline_edges lib/neatogen/neatosplines.c:632
#3 0x7f5b4452b4c8 in splineEdges lib/neatogen/neatosplines.c:729
#4 0x7f5b4452b5a6 in spline_edges1 lib/neatogen/neatosplines.c:742
#5 0x7f5b4452b658 in spline_edges0 lib/neatogen/neatosplines.c:771
#6 0x7f5b4451154d in init_nop lib/neatogen/neatoinit.c:597
#7 0x7f5b44516ac9 in neato_layout lib/neatogen/neatoinit.c:1407
#8 0x7f5b488a847f (/tmp/tmp.jiRlFO5wtW/lib+0x8647f)
#9 0x55ddbc8126dc in main graphviz/cmd/dot/dot.c:89
#10 0x7f5b485e2d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
#11 0x7f5b485e2e3f in __libc_start_main_impl ../csu/libc-start.c:392
#12 0x55ddbc812344 in _start (bin/dot+0x1344)
It should be clear from the above that the code here has a very error prone
structure. (1) Estimating how much memory is required upfront with non-trivial
calculations and (2) allocating a block of memory that is then partitioned out
to clients trusted not to run into each other, effectively creates a scenario
where bugs are undetectable by memory safety tools and easily compound one
another. In future the other allocations in this file should be rewritten to
avoid this structure too.
The code added in 24bd92c1e5d49b354141cd06d88ca658991b9825 calls `readlink` in
multiple paths, including the macOS branch. This function needs the #include
<unistd.h> but we were only conditionally including it when not on Windows or
macOS, when we _do_ need it on macOS. Similar to the previous commit, it seems
this went unnoticed because the macOS CI jobs do not build Smyrna.
24bd92c1e5d49b354141cd06d88ca658991b9825 managed to commit code to Smyrna on
which the macOS branch of the logic does not compile. It seems this went
unnoticed because the macOS CI jobs do not build Smyrna.
use input gradient angle rather than computed gradient angle when checking for 0
When using radial gradients, a value of 0 is interpreted differently to other
values:¹
a value of zero causes the colors to transform radially from the center; for
non-zero values, the colors transform from a point near the object’s periphery
as specified by the value.
The code affected in this commit is looking for this situation, but was
unnecessarily using the computed gradient angle. If the original is zero, the
computed angle will also be zero. So we can simplify this code for both human
readers and the compiler by using the original (integer) angle.
This squashes the warnings:
gvrender_core_dot.c: In function ‘xdot_gradient_fillcolor’:
gvrender_core_dot.c:644:19: warning: comparing floating-point with ‘==’ or
‘!=’ is unsafe [-Wfloat-equal]
644 | if (angle == 0) {
| ^~
gvrender_core_svg.c: In function ‘svg_rgradstyle’:
gvrender_core_svg.c:602:15: warning: comparing floating-point with ‘==’ or
‘!=’ is unsafe [-Wfloat-equal]
602 | if (angle == 0.) {
| ^~
gvrender_pango.c:322:19: warning: comparing floating-point with ‘==’ or ‘!=’
is unsafe [-Wfloat-equal]
322 | if (angle == 0) {
| ^~
This has been wrong since 0e19cdb9bff60fdeb1942f5dca677db9d8a24062 removed the
rtest directory. And yet the tests in this file somehow still run correctly in
CI. I can only guess that insufficient test isolation under Pytest results in
some other test file putting the correct path in `sys.path` prior to this file
being processed.
Pierre Labastie [Tue, 15 Nov 2022 08:03:54 +0000 (09:03 +0100)]
Fix php install for SWIG-4.0.1
This version of SWIG does not generate gv.php anymore, so
installing it shouldn't be tried. The autotools framework is
already there, so just use a conditional in Makefile.am.
The test job was doing this but the build job was not. Technically I think this
is unnecessary (I think CMake includes `CFLAGS` or `CXXFLAGS` when calling `ld`)
but it seems worth being consistent.
CI: make UBSan errors in compiled test cases fail the test
0d0c20797a0cf2937886945f3aecc4e8a9a55e8c adjusted this for the build job, but
not for the test job. It is useful to have this enabled for the test job as well
because some test cases involve compiling C code, for which we want full ASan
and UBSan enabled too.
shortest.c: In function ‘growtris’:
shortest.c:518:39: warning: conversion to ‘long unsigned int’ from ‘int’ may
change the sign of the result [-Wsign-conversion]
518 | tris = realloc(tris, TRIANGLESIZE * newtrin);
| ^
At the call site for `growtris`, it is not clear to me why `trin + 20` is
enough. That is, it seems like this branch can be hit, reallocation can succeed,
but `tril` was greater than `trin + 20` so the array remains too small for the
follow on computation. But this is a separate problem.
pathplan: use a 'size_t' for counting allocated points
Squashes:
shortest.c:500:41: warning: conversion to ‘long unsigned int’ from ‘int’ may
change the sign of the result [-Wsign-conversion]
500 | pnls = realloc(pnls, POINTNLINKSIZE * newpnln);
| ^
shortest.c:505:44: warning: conversion to ‘long unsigned int’ from ‘int’ may
change the sign of the result [-Wsign-conversion]
505 | pnlps = realloc(pnlps, POINTNLINKPSIZE * newpnln);
| ^
It would be nice to adjust `Ppoly_t.pn` to avoid the casting, but it is part of
the public API and it does not seem worth an API break for this.
pathplan: phrase comparison in 'splinefits' with a tolerance
From the adjustments to `a` in this function, it can be seen this is equivalent.
This squashes the warning:
route.c: In function ‘splinefits’:
route.c:252:15: warning: comparing floating-point with ‘==’ or ‘!=’ is unsafe
[-Wfloat-equal]
252 | if (a == 0) {
| ^~
This squashes the following warnings on CentOS 7 where the compiler believes
the sequence ID may overflow:
node.c: In function 'agnodebefore':
node.c:370:23: warning: conversion to 'unsigned int:28' from 'int' may alter
its value [-Wconversion]
AGSEQ(n) = AGSEQ(n) + 1;
^
node.c:376:26: warning: conversion to 'unsigned int:28' from 'int' may alter
its value [-Wconversion]
AGSEQ(snd) = AGSEQ(fst) - 1;
^
This attempts to replicate how this target is defined in the Autotools build
system, but involved some amount of guesswork because the Autotools build system
is not explicit with _why_ it is adding particular dependencies to link/include
lists.
The rpath tweak in this commit required surprisingly lengthy debugging to arrive
at. I am still not completely confident it is correct nor that this is the right
way to do such things.
dot_builtins: use 'dllimport' even when LTDL is in use
I am not sure why 53eec6fb96646da693e5e8f78047cccf91774dbb included LTDL in the
conditional here. Whether LTDL is in use or not has no bearing on whether we
`dllimport` mark the plugin libraries, because we are unconditionally
dynamically linking them (as opposed to LTDL-based on demand loading).
CMake: fix: add Zlib include directories when building with Zlib
This worked out before because everywhere we test has Zlib installed in the
default system paths. But technically this should be setup in order to correctly
use a Zlib installed somewhere else.
This code is trying to abbreviate printing `0.000` or `1.000`. We can more
accurately describe the situations under which this will happen, squashing:
gvrender_core_svg.c: In function ‘svg_print_stop’:
gvrender_core_svg.c:525:16: warning: comparing floating-point with ‘==’ or
‘!=’ is unsafe [-Wfloat-equal]
525 | if (offset == 0.0)
| ^~
gvrender_core_svg.c:527:21: warning: comparing floating-point with ‘==’ or
‘!=’ is unsafe [-Wfloat-equal]
527 | else if (offset == 1.0)
| ^~
`gvprintdouble` prints to two decimal places. So we can take this into account
when deciding whether we need to print a non-default pen width, squashing:
gvrender_core_svg.c: In function ‘svg_grstyle’:
gvrender_core_svg.c:191:23: warning: comparing floating-point with ‘==’ or
‘!=’ is unsafe [-Wfloat-equal]
191 | if (obj->penwidth != PENWIDTH_NORMAL) {
| ^~
projects / graphviz / log