remove CreateHyperlink

This function was adding no value over just calling the Hyperlink constructor
directly.

manage Hyperlinks by value instead of pointer in the visio plugin

There is no inheritance involved here, so there was no advantage to manually
managing memory for Hyperlinks or passing them around by pointer.

fix: use managed memory for Hyperlink’s string pointers

Similar to the prior fix to Run, this removes a latent safety issue with the
management of these pointers.

manage _run member of Text as a value instead of a pointer

This member is never set to NULL and the Run class is not involved in any
inheritance hierarchy. So there is no need to heap-allocate it or pass it around
by pointer.

fix: use managed memory for Run’s _text member

This removes the need to manually manage memory for this member but, more
importantly makes this class copy-safe. The implicit copy constructor of this
class would copy the pointer in the _text member. When either the original
object or the new copy was deleted, the pointer would be freed leaving its
duplicate in the other copy dangling. Any attempt to access this would result in
use-after-free and deleting the object would cause a double-free.

Following this change, it is safe to copy a Run object. This issue was latent,
because no existing code causes a Run object to be copied.

Merge branch 'smattr/ad4aa51f-c3fd-4ba1-97a5-61d0fba501eb' into 'main'

some warning clean up in cmd/smyrna

See merge request graphviz/graphviz!1898

squash -Wmissing-prototypes warning

remove unused parameter from glmotion_zoom

remove unused inc_value variable in glmotion_zoom_inc

remove unused Status variable

This was only used by LoadGLTextures that was recently removed.

squash a -Wfloat-conversion warning

remove unused LoadGLTextures

squash -Wunused-parameter in EmbedText

squash -Wunused-parameter warning in InsertImage

squash -Wunused-parameter warning in SetFont

squash -Wunused-parameter warnings in SetStyle

squash -Wunused-parameter in SetPenColor

squash -Wunused-parameter warning in SetFillColor

fix -Wpointer-compare warning

remove unused filled variable from DrawPolygon

remove unused parameter from set_options

squash a -Wstrict-prototypes warning

remove unused parameter from arcmouseDrag

remove unused parameter from arcmouseClick

remove unused code

Merge branch 'smattr/5EDB9D68-064A-4027-AFC3-A46101BAF4E5' into 'main'

in Windows CI tasks, install Python 3 from Chocolatey

See merge request graphviz/graphviz!1896

remove Python 3.8 fallback in Windows CI tasks

As of the previous commit, Python >3.8 is installed prior to running this
script, so no fallback should be required. Python 3.8 can be removed from the
Windows build utilities repository.¹

  ¹ https://gitlab.com/graphviz/graphviz-build-utilities

install Python 3 as a pre-requisite in Windows CI tasks

Prior to this change, Python 3.8 was used on Windows in CI via the Windows
build dependencies repository.¹ This works but has a number of shortcomings:

  1. Upgrading the version of Python used in CI requires access to a Windows
     machine; and
  2. Upgrading the version of Python used in CI requires building CPython from
     source; and
  3. Python-dependent failures are hard to root cause because they may be an
     issue with how we built CPython.²

As of this commit, Python 3 is installed via the Chocolatey³ package manager.
At time of writing, this installs Python 3.9.4. This is not pinned to a specific
version – i.e. a new Chocolatey package of Python 3 will result in Graphviz
immediately using that in CI. I believe this is actually the behavior we want
right now; pro-actively catching any issues caused by Python upgrades.

This does not change the baseline requirement for Graphviz CI/test scripts from
Python 3.6. Scripts should still be written to be backwards compatible to 3.6.

As this change introduces a new non-trivial step in the Windows CI tasks,
execution time is relevant. At time of writing, the macOS jobs dominate Graphviz
CI execution time, both before and after this change. Thus I think this is
acceptable.

  ¹ https://gitlab.com/graphviz/graphviz-build-utilities
  ² E.g. #2000, “CI Python OSErrors from Windows tasks”
  ³ https://chocolatey.org/

Merge branch 'smattr/d3552219-a755-4a56-9a55-2eebb5819810' into 'main'

more vpsc clean up

See merge request graphviz/graphviz!1895

push a variable closer its use in generateYConstraints

push a variable closer to its use in generateXConstraints

use range-based for loops to clean up some iteration

cut variable dependency and push closer to its use in generateYConstraints

cut variable dependency and push closer to its use in generateXConstraints

remove unnecessary manual indexing in generateYConstraints

It’s a little hard to see because the variable i is also shadowed within this
loop, but the loop index is unnecessary.

remove unnecessary manual indexing in generateXConstraints

It’s a little hard to see because the variable i is also shadowed within this
loop, but the loop index is unnecessary.

manage events collections in vpsc as values instead of pointers

This removes some manual memory management and optimizes the layout of these
elements (note emplace_back). Memory for individual events is now retained
slightly longer because a manual deletion of pointers event-by-event was removed
and we now rely on the final vector clean up to free this memory. But the impact
of this is expected to be negligible and outweighed by the improved layout.

replace some qsort calls with std::sort

This is safer and has the potential to be more efficient. Note that this
required rewriting the involved comparator to judge less-than instead of a
three-outcome comparison.

replace a manual array with a vector in generateYConstraints

There is no need to manually manage memory for this collection.

replace a manual array with a vector in generateXConstraints

There is no need to manually manage memory for this collection.

make events a local variable in generateXConstraints and generateYConstraints

Not only is this variable not used outside its containing file (could be
declared static), but its lifetime within both these functions is entirely
contained. Reusing the same global in these functions has no advantage, but
impairs the compiler’s ability to optimize by potentially confusing its alias
analysis.

Merge branch 'smattr/187F4EB8-B1D4-4C94-B167-3B7BFDA60D06' into 'main'

more sfstropen removal

See merge request graphviz/graphviz!1894

remove some unnecessary bracketing

fix some Windows detection in libexpr

This silences some -Wundef warnings on non-Windows but, more importantly, makes
Windows environments more reliably see the code intended for them.

squash a -Wsign-compare warning

squash a -Wunused-parameter

squash a -Wunused-parameter warning

emove use of persistent buffer in extokens

This removes a layer of indirection. Previously data was being written to the
persistent scratch space, ex->tmp, and then copied to a further temporary buffer
in tok. We now simply allocate to tok and write directly into it. This also
addresses an issue where the persistent buffer was being unconditionally used,
despite that it may have failed to be opened due to lack of memory.

Related to #1873.

remove use of persistent buffer in exsplit

This removes a layer of indirection. Previously data was being written to the
persistent scratch space, ex->tmp, and then copied to a further temporary buffer
in tok. We now simply allocate to tok and write directly into it. This also
addresses an issue where the persistent buffer was being unconditionally used,
despite that it may have failed to be opened due to lack of memory.

Related to #1873.

use a local buffer for expr sprintf instead of a persistent one

Though this introduces another call to sfstropen, it is actually a step towards
removing sfstropen calls. The first part of this is localizing existing usages;
avoiding the ex->tmp buffer. Then the implementation of expr’s sprintf itself
will need to be refactored to operate on a non-SFIO buffer.

Note that this also addresses an issue where expr’s sprintf would use the
persistent buffer even if prior opening of it had failed due to lack of memory.

Related to #1873.

remove use of persistent buffer in str_add

Related to #1873.

remove some unnecessary casts

These pointers implicitly coerce.

Merge branch 'smattr/71d79a68-7f10-496c-bc4b-41406129743b' into 'main'

remove another use of setjmp/longjmp

See merge request graphviz/graphviz!1893

remove now unused jmp buffer

Having unraveled the use of longjmp here, the jmp_buf and its setup are no
longer required.

On GCC 8.3.0 this removes a -Wclobbered warning. Though interestingly the
warning text itself is a false positive, referencing nverts that is not used in
the setjmp(…)!=0 path. The actual dangerously used variables in this scenario
were polygon_list and vertex_list. My take away here is that *any* -Wclobbered
warning is typically an indicator of incorrect code. I have never seen a
-Wclobbered warning (from any compiler) for code that was not buggy.

Related to #1801.

return an error value instead of longjmping out of find_ints

Related to #1801.

anticipate <0 meaning failure from find_ints

find_ints does not currently return negative values. It returns only 0 or 1.
However, we would like to unravel a use of longjmp within it, so returning a
negative value to indicate an error is convenient. This is preparation for
upcoming changes. Related to #1801.

fix comment typo

Merge branch 'smattr/a2c0e10b-3770-4314-9761-004b3181b192' into 'main'

observe CFLAGS and LDFLAGS when compiling C code during testing

See merge request graphviz/graphviz!1903

echo C compiler commands in the test suite

If a test compiles and/or runs C code and that test fails, included in the test
suite output will now be the commands themselves that were run, echoed in the
style of Bash’s set -x.

fix a Python type signature

Technically List[str] is the type of a list of strings, not [str].

support receiving C compiler flags in the test suite from the environment

The environment variables CFLAGS and LDFLAGS now also effect the flags used with
the C compiler in the test suite, in addition to the build system. Related to
#1881.

use general C compiler support for vmalloc test

Now that the test suite’s C compiler support also allows passing extra flags,
the vmalloc test no longer needs to do its work manually.

support passing extra args to the test suite’s C compiler interface

consolidate common test suite functionality

In future we could grow this gvtest.py to be a more full featured library of
common functions we need in the Graphviz test suite.

Merge branch 'smattr/2B83BB5F-5D3B-4537-A875-E8225842E9C7' into 'main'

visio plugin: further pointer and manual memory management clean up

See merge request graphviz/graphviz!1889

visio plugin: manage _chars member of Text as a value instead of a pointer

This member is never set to NULL and the Char class is not involved in any
inheritance hierarchy. So there is no need to heap-allocate it or pass it around
by pointer.

visio plugin: manage _para member of Text as a value instead of a pointer

This member is never set to NULL and the Para class is not involved in any
inheritance hierarchy. So there is no need to heap-allocate it or pass it around
by pointer.

visio plugin: use more modern #pragma once guards

visio plugin: manage _fill member of Graphic as a smart pointer

visio plugin: manage _geom member of Graphic as a smart pointer

visio plugin: manage _line member of Graphic as a value instead of pointer

This member is never set to NULL and the Line class is not involved in any
inheritance hierarchy. So there is no need to heap-allocate it or pass it around
by pointer.

Merge branch 'smattr/93449861-eaa3-4072-a1fc-0f7aa57f77eb' into 'main'

remove unnecessary lib/ast function

See merge request graphviz/graphviz!1892

remove now no longer used pathbin.c

inline a getenv accessor and use a more appropriate return type

remove non-portable default for $PATH

Across the range of platforms Graphviz runs on, this is not a reasonable
default. E.g. Windows will have none of these paths and this is not even how one
separates paths in Windows $PATH. This is mostly a latent issue, as it is very
rare for $PATH to be unset.

remove getenv("PATH") caching

On most modern platforms, getenv is accessing a static array in the address
space. So this caching does not add any benefit.

Merge branch 'smattr/AB0E37EB-52FF-4036-8BF3-F01425B7486E' into 'main'

remove astquery prototype

See merge request graphviz/graphviz!1891

remove astquery prototype

This function is not implemented.

Merge branch 'smattr/ca8249f0-859f-412b-832d-55cdf8d7bab9' into 'main'

squash some compiler warnings in lib/ortho

See merge request graphviz/graphviz!1890

remove commented out code

remove unused parameter e from addEdgeToNode

remove some commented out code

track edge counts as size_t in ortho.c

This fixes some -Wsign-conversion warnings at the expense of introducing some
-Wsign-compare warnings. The latter should be removed as we migrate the cell
count in mazes to size_t.

remove unused parameter ix to emitEdge

remove an unnecessary cast

represent route array lengths as size_t

This fixes one -Wsign-conversion warning, though introduces a -Wconversion
warning. We should be able to remove the latter as we continue cleaning up the
use of int for sizes.

remove some commented out code

squash a -Wsign-conversion warning

squash a -Wmissing-prototypes warning

squash some -Wunused-parameter warnings

squash two -Wswitch-default warnings

squash a -Wswitch-default warning

squash a -Wswitch-default warning

squash some -Wunused-parameter warnings

use a stronger type to squash a -Wconversion warning

remove unused parameter route_list from assignTracks

remove unused parameter nrtes from assignTracks

Merge branch 'smattr/CC513531-8BFC-4587-8816-4E41C4672694' into 'main'

some clean up of sfextern.c

See merge request graphviz/graphviz!1888

swap some use of hand rolled NIL for the standard NULL

remove unnecessary global initialization

This structure is default-0-initialized.