Merge branch 'smattr/9ca68f13-5591-4f3c-a488-f3b15c827039' into 'main'

fix incorrect reference counting of HTML-like strings in gvpack

See merge request graphviz/graphviz!2026

fix incorrect reference counting of HTML-like strings in gvpack

When duplicating graph attributes, gvpack was calling agstrdup_html to construct
an HTML-like string and then agattr/agset to assign this to an attribute. It did
not anticipate that *both* of these increment the reference count for the string
that has been interned in the graph’s dictionary. The result was a reference
count of 2 for a new HTML-like string, despite only 1 reference to the string
existing.

This was not directly visible to users (hence no changelog entry for this
commit) and the extra reference counts are disposed of when the graph itself is
destroyed. However, this persistent off-by-one issue internally confused some
debugging and potentially retained memory longer than was necessary. The issue
was visible with the following patch applied:

  diff --git cmd/tools/gvpack.c cmd/tools/gvpack.c
  index 16a4ffa5b..8e8fbf3b9 100644
  --- cmd/tools/gvpack.c
  +++ cmd/tools/gvpack.c
  @@ -352,9 +352,13 @@ static void cloneAttrs(void *old, void *new)

       for (a = agnxtattr(g, kind, 0); a; a =  agnxtattr(g, kind, a)) {
    s = agxget (old, a);
  - if (aghtmlstr(s))
  -     agset(new, a->name, agstrdup_html(ng, s));
  - else
  + if (aghtmlstr(s)) {
  +     printf("creating HTML string...\n");
  +     char *scopy = agstrdup_html(ng, s);
  +     agrefcntprint(scopy);
  +     agset(new, a->name, scopy);
  +     agrefcntprint(scopy);
  + } else
        agset(new, a->name, s);
       }
   }
  diff --git lib/cgraph/cgraph.h lib/cgraph/cgraph.h
  index a043fb26d..e2b065e16 100644
  --- lib/cgraph/cgraph.h
  +++ lib/cgraph/cgraph.h
  @@ -305,6 +305,7 @@ CGRAPH_API int agobjkind(void *);
   /* strings */
   CGRAPH_API char *agstrdup(Agraph_t *, char *);
   CGRAPH_API char *agstrdup_html(Agraph_t *, char *);
  +CGRAPH_API void *agrefcntprint(const char *);
   CGRAPH_API int aghtmlstr(char *);
   CGRAPH_API char *agstrbind(Agraph_t * g, char *);
   CGRAPH_API int agstrfree(Agraph_t *, char *);
  diff --git lib/cgraph/refstr.c lib/cgraph/refstr.c
  index b524649ff..d8d7ca468 100644
  --- lib/cgraph/refstr.c
  +++ lib/cgraph/refstr.c
  @@ -9,6 +9,7 @@
    *************************************************************************/

   #include <cgraph/cghdr.h>
  +#include <inttypes.h>
   #include <stddef.h>
   #include <stdio.h>

  @@ -139,6 +140,11 @@ char *agstrdup_html(Agraph_t * g, char *s)
       return r->s;
   }

  +void *agrefcntprint(const char *s) {
  +    refstr_t *key = (refstr_t *) (s - offsetof(refstr_t, store[0]));
  +    printf("reference count: %" PRIu64 "\n", (uint64_t)key->refcnt);
  +}
  +
   int agstrfree(Agraph_t * g, char *s)
   {
       refstr_t *r;

With these changes, attempting to gvpack a graph with an HTML-like string
attribute results in the following output:

  creating HTML string...
  reference count: 1
  reference count: 2

The present commit fixes this issue by replicating the correct pattern for doing
this kind of duplication from lib/gvpr/actions.c:copyAttr.

Merge branch 'smattr/32e830af-ed59-441c-9dc6-9eaef9316b04' into 'main'

work towards solution for round tripping GV→GXL→GV

See merge request graphviz/graphviz!2016

remove some unnecessary casts to agxbuf functions

These functions take const char pointers. There is no need to cast away the
const-ness of the original pointers.

gv2gxl: in GXL output, indicate when a label originated as an HTML like label

When translating Dot to GXL, labels come out as strings. Input something like:
`label="foo"` and the GXL output is something like
`<attr name="label"><string>foo</string></attr>`. This translation is fine and
works well, until you start using HTML like labels.

Graphviz allows `<` and `>` as delimiters to indicate your label string contains
HTML that should be parsed. When you try to use this feature in combination with
gv2gxl, you get a lossy translation. E.g. the labels `label="foo"` and
`label=<foo>` come out identically in GXL.

This commit makes use of the GXL ‘kind’ field on attributes to note that the
source was an HTML like label. If you translate the two labels from above, you
will now get:

  <attr name="label"><string>foo</string></attr>

and

  <attr name="label" kind="HTML-like string"><string>foo</string></attr>

respectively.

This is a partial fix for #517. This makes the translation to GXL no longer
lossy. The other half of this is recognizing the ‘kind’ field during gxl2gv
translation and preserving it on the way back.

reflow some text

add a test case for #517

Merge branch 'smattr/cbc230d2-1896-4082-84b8-df235572234c' into 'main'

branch-tune allocation wrappers

See merge request graphviz/graphviz!2021

branch-tune allocation wrappers

These functions are used extensively within Graphviz. This change tells the
compiler to optimize for the case when allocation succeeds. This may slow the
case when allocation fails, though we do not care about this case as Graphviz is
about to exit with failure in this scenario.

Merge branch 'smattr/5d05e2d1-45cd-4422-b78c-122a62fff5c8' into 'main'

more gvmap-related clean up

See merge request graphviz/graphviz!2025

remove GVIEWER-guarded code

Nothing in the build system(s) define this. A lot of this also looks broken.
For example, the GVIEWER alternative of the stress_model function sets up an
argv whose first string entry is not terminated.

remove commented out code in stress_model.c

remove useless flag parameter to remove_overlap

This function has no path in which it sets this output parameter to non-zero.

mark array parameters to get_local_12_norm as const

This makes it clearer that this function does not modify these arrays.

remove unused norm variable in improve_antibandwidth_by_swapping

remove unused norm_1 output parameter to country_graph_coloring_internal

remove unused norm_1 output parameter to country_graph_coloring

Merge branch 'smattr/B7DD3AE0-829A-49E5-9A94-5518A9FFA6D9' into 'main'

remove unused gvfwrite

Closes #1846

See merge request graphviz/graphviz!2029

remove unused gvfwrite

The last use of gvfwrite was removed in
6e818420cf02ad01d9c976b69d49fb794a661f5e. But more problematically, this
function seems incorrect. By ignoring its size argument it will generate
surprising results if ever called with a non-1 size. Closes #1846.

Merge branch 'smattr/12de6328-7c30-45ad-9029-63f88edd6385' into 'main'

clean up in lib/sfio

See merge request graphviz/graphviz!2027

Merge branch 'smattr/F16DEFA3-B7E0-4DC8-BDD5-C4D2C9D8242F' into 'main'

port deploy script from os.path to pathlib

See merge request graphviz/graphviz!2024

remove _SFBINARY_H-conditional code

Nothing in the build defines this.

remove unused SETCLOEXEC and FD_CLOEXEC

The last use of these was removed in f552c0dece1435773de62334dee3310d1728e22f.

remove unused ESPIPE

ESPIPE has never been used in sfio.

remove unused ENOMEM

The last use of this in sfio was removed in
bb04aad8c6e033b6fef9cecd424a4d02758d661a.

remove cbrt configuration check

Graphviz is built with C99 which requires cbrt.

remove unlink configuration check and Windows shim

Nothing in Graphviz uses unlink.

remove remove/unlink declarations in sfio

The last use of these was removed in 751ab539a2531b5459ef709946ea0642ce2c44e8.

remove fork/vfork detection

As of the previous commit, nothing uses these configuration variables.

remove unused sfio fork/vfork declarations

The last usage of fork in this library was removed in
f552c0dece1435773de62334dee3310d1728e22f.

remove unnecessary errno declaration

The errno variable/macro is part of ISO C.

remove unused MAXSHORT

remove unused MAXLONG

squash Pylint warning in deploy script

With this tweak, deploy.py is Pylint-warning-free. I do not think it makes sense
to write docstrings on a main function.

port deploy script from os.path to pathlib

While this does not look like a win in terms of conciseness, I think the trade
off is worth it for stronger typing within this script. It is hard to test this
script without actually running a deployment, so we heavily rely on Pylint to
validate what it is doing. Giving Pylint some more hints as to what is going on
seems like a good idea.

Merge branch 'smattr/77f377e7-e733-48ca-8003-6de107b48820' into 'main'

test cases for #2089

See merge request graphviz/graphviz!2023

test cases for #2089

remove prototype for unimplemented agrelabel

I do not know why this function was described as “scary.” However, it seems it
was so scary no one in Graphviz’ history ever implemented it.

Merge branch 'smattr/259855E7-1FFB-4A36-8116-EA81D402CFCD' into 'main'

fix macOS CI

See merge request graphviz/graphviz!2030

remove Courier.gv graph reference test

This test compares the rendering of a graph using the Courier font against
reference examples. However the rendering of Courier is not stable across
operating system releases. At present, this test is spuriously failing due to a
macOS upgrade. But in principle, an upgrade of any platform can break it. To
avoid testing this unstable behavior, we remove this test.

remove unused font_tests.txt

This seems to be a subset of tests covered elsewhere. This file is also
currently unused.

Update README.md -- queries go to Graphviz forum

Should keep bug tracker full of bugs, and give people with other queries a better chance at getting them answered (there are more people on the forum).

Merge branch 'smattr/94045bf0-ac99-4edb-acba-84c65c67d983' into 'main'

clean up in lib/vpsc

See merge request graphviz/graphviz!2020

remove unused Rectangle {x|y}Border

use a range-based for-loop to simplify remapOutConstraints

use a range-based for-loop to simplify remapInConstraints

rephrase accumulate loop in Blocks::cost using a range-based for-loop

rephrase delete loop in Blocks::~Blocks to a range-based for-loop

remove unnecessary clear of Blocks set

The object is being destructed, so the underlying set is automatically cleared.

remove unused getSplitCnt

remove unused splitIncVPSC

remove duplicate solve_VPSC.h in VPSC noinst_HEADERS list

remove unused newVPSC

Merge branch 'smattr/49e76f9c-dbd6-42d8-bcc1-cbe0a523441e' into 'main'

clean up in lib/sparse

See merge request graphviz/graphviz!2019

remove unused StringVector

remove unused IntegerVector

remove unused SparseMatrix_pseudo_diameter_weighted

remove unused SparseMatrix_pseudo_diameter_unweighted

remove pattern_symmetric_only parameter to SparseMatrix_symmetrize_nodiag

This function is only called with FALSE for this argument, so we can propagate
it through and remove the parameter.

remove duplicate palette_grey_to_red

remove another unused MAXINT

remove unused MAXINT

remove unused vector_subtract_from

remove unused vector_add_to

Merge branch 'smattr/584ca143-13f9-4b18-aecc-1195d5de7644' into 'main'

clean up in lib/common

See merge request graphviz/graphviz!2018

reflow some routespl.c code for readability

remove dead code in get_centroid

The preceding return statement ensures none of the latter part of the function
is reachable. This was introduced in 05674ae147c14ec4ec74b2b6771e04e0c82209b8
and seemingly never noticed.

This commit also drops now-unused variables, as well as the static qualifier
from sum. It is not clear to me what the purpose of making sum static was. It is
a small struct and both its fields are overwritten immediately when entering
this function, so there is no value to retaining it across calls or
BSS-allocating it.

remove a micro-optimization in checkpath

Self-stores like this are effectively free on a modern CPU. The branch guarding
this store is more costly than the store itself.

use C99 booleans to clarify some ambiguous types in routespl.c

remove unnecessary bracketing in lib/common/routespl.c

remove casts of vec_get’s return value

The vec_get function returns a void* that implicitly coerces to other pointer
types.

condense some compound assignments

remove __CYCLE_CENTROID and conditional code for when it is undefined

This was unconditionally defined, so remove the alternative that is always
disabled.

rephrase open coded fmax/fmin in checkpath

_routesplines: remove unnecessary cast

pp->data is a void*, which implicitly coerces to a edge_t*.

remove commented out code in lib/common

remove DONTFIXPATH guards

These guards were present in the very first revision of Graphviz, but nothing
has ever defined them.

remove NOTNOW-guarded code

Nothing in the build defines this.

Merge branch 'fix-typos-GD_LIBS-to-GDLIB_LIBS' into 'main'

Fix a typo GD_LIBS to GDLIB_LIBS in tclpkg/tcldot/Makefile.am

See merge request graphviz/graphviz!2022

add a CHANGELOG.md entry for the GTK→GDLIB change

This fix and the GD_LIBS→GDLIB_LIBS fix are both addressing problems that were
introduced in 3fcf096870bffa7d21fceecb56ba26e925073ecb.

move GD_LIBS typo fix to Fixes section of CHANGELOG.md

realign configure.ac after removal of $with_gd conditional

Avoid cases where `$with_libgd = yes` in spite of `$use_gd != Yes`

Such cases would have occurred when
the first `test "x$use_gd" = "x"` [1] is true
and then the third `test "x$use_gd" = "x"` [2] is false.

[1]: Just before `PKG_CHECK_MODULES([GDLIB], [gdlib >= 2.0.33],[`
[2]: Just before `# see if we can use the external gd lib`

Remove a redundant `if`-conditional, which is always true in its scope

Fix a typo `GDK` to `[GDLIB]` in a call `PKG_CHECK_MODULES()`

Remove a redundant arg `$GD_LIBS`

It might be a typo of `$GDLIB_LIBS`,
and, if so, it is redundant, since `$LDFLAGS` contains `$GDLIB_LIBS`.

Update CHANGELOG.md

For https://gitlab.com/graphviz/graphviz/-/merge_requests/2022

Fix a typo GD_LIBS to GDLIB_LIBS in tclpkg/tcldot/Makefile.am

Merge branch 'smattr/EFF6337A-A391-409A-9B11-F11C26A76481' into 'main'

CMake: when finding Bison, require ≥3.0

Closes #1916

See merge request graphviz/graphviz!2008

CMake: when finding Bison, require ≥3.0

Commit 4a13c9598ad3d950a251e5d3e5be8d05c59c6214 made the minimum Bison baseline
3.0. This change teaches CMake ths constraint, so using a version of Bison <3.0
now results in a clearer error during configuration rather than a cryptic
failure during build. Closes #1916.

Merge branch 'smattr/5c6b4c6f-b22b-49a6-8607-7a4b5caa120a' into 'main'

clean up in lib/edgepaint

See merge request graphviz/graphviz!2007

remove color_diff and color_diff_sum computation from node_distinct_coloring

Neither of the callers of this function use the results returned in these output
parameters.

remove commented out code in node_distinct_coloring.c

remove some variable shadowing in node_distinct_coloring_internal2

Squashes two -Wshadow warnings.

remove unused x parameter to splines_intersect

remove unused color_blend_rgb

remove unused color_blend_rgbstring

return errors from node_distinct_coloring via the return value, rather than flag

This makes things simpler for readers, callers, as well as the compiler.

remove commented out code in edge_distinct_coloring.c

remove unused lab_gamut_from_file