The unusual structure of the loop in this function made it appear as if it was
intending to cope with e.g. an `n` of 0 but still print the first element of
`p`. Surveying the callers of `gvprintpointflist`, we can see it is never used
this way. So we can write it simpler and more readably.
The `$GV_FILE_PATH` environment variable could be set to sandbox Graphviz’
ability to read and write to the file system. This made sense once upon a time,
but the world around Graphviz has shifted. Sandboxing yourself is no longer as
valuable a proposition as an external sandboxer that can be more easily audited.
Platforms’ ecosystems have matured to support this use case (Capsicum on
FreeBSD, Seccomp on Linux, App Sandbox on macOS, Pledge on OpenBSD, …).
This change makes any attempt to use `$GV_FILE_PATH` “fail-closed,” in the
sense that execution will be aborted. This may be surprising and not what the
user intended, but this conservatively guarantees safety: you can never think
you have enabled `$GV_FILE_PATH`-based sandboxing and be instead running
unguarded.
1d28d7d2b4d2b2551bd1f432aa175f54a69364a4 appears to have copy-pasted much of the
initial code from cmd/tools/cvtgxl.c, accidentally retaining this usage message
referring to the wrong tool.
This makes very little difference to the compiler, as it can see all uses of
these strings. But this makes it clearer to readers that these variables are not
modified. Using an array instead of a pointer slightly increases flexibility,
allowing a useful application of `sizeof` to these variables should we ever want
to do that.
DEVELOPERS.md: move how to make a release to the bottom
This guide was sort of back-to-front, in that it prioritized readers who were
maintainers looking at cutting an upcoming release. In reality, this document is
read far more often by drive-by contributors seeking how to build and test their
changes.
DEVELOPERS.md: drop TODO about updating example commits
Despite coming up on 5.0.1, we have not updated the example commits since
2.44.1. This seems like strong evidence that the existing examples are
sufficient.
In a sort of Chesterton’s fence¹ sense, I do not understand the existing code.
While I understand the first order motivation (dynamic string buffering inline),
I do not follow the implementation. It manages to hit the trifecta:
1. NULL pointer dereference. Failures of allocation functions were not
checked.
2. Memory leaks. Calls to `agstrdup` and `agstrdup_html` allocate an owned
string internally, yet this code was acting as if they stole the `Sbuf`
backing memory.
3. Not-obviously-correct and repeated computation of the end of the buffer to
append to. `addstr` seemed overly complex and inefficient.
Static initialization of an agxbuf was a bit complicated until 04a248348798a4a32fc9ac7e5d860a84ab5ee49f, though it was still possible.
Following that commit, it becomes a no-brainer.
The code after this change still leaks memory. Perhaps in future we should reset
(`agxbfree(&Sbuf); memset(&Sbuf, 0, sizeof(Sbuf));`) the buffer after `agstrdup`
or `agstrdup_html`, though there would be a performance cost to this.
Magnus Jacobsson [Thu, 21 Jul 2022 11:16:41 +0000 (13:16 +0200)]
tests: SvgAnalyzer: add internal re-creation of the SVG element hierarchy
This will be used in upcoming commits in this series to store
attributes on and to re-create the original SVG from. Also, an
upcoming commit series will extend the SVG analyzer to be aware of
Graphviz graphs, nodes and edges and will use this hierarchy and the
associated attributes to be able to answer questions about their
properties.
tests: svgAnalyzer: remove useless handling of 'any' SVG element
We haven't asked SVG++ to be called back for any element, just for
explicitly specified elements. Therefore we will never receive any
unknown elements that the SVG analyzer could count so this
functionality is also removed.
gvpr colorx: use a local agxbuf instead of GVPR state’s 'tmp'
This makes it clearer to both humans and the compiler that the data written to
this buffer is not required beyond this function. This also gets us closer to
removing an SFIO dependency.
This makes it clearer to both humans and the compiler that the data written to
this buffer has a much more constrained scope than was previously implied. This
also gets us closer to removing an SFIO dependency.
gvpr traverse: use a local agxbuf instead of GVPR state’s 'tmp'
This makes it clearer to both humans and the compiler that the data written to
this buffer is not required beyond this function. This also gets us closer to
removing an SFIO dependency.
`toUpper` worked by using the GVPR state’s temporary buffer, `tmp`, to
incrementally construct an uppercase copy of the input. This works but is
suboptimal. When printing to the temporary buffer, it has no knowledge of what
we are up to and needs to make heuristic-based decisions on how to allocate
memory. But we know the exact size of the result string already. This commit
takes advantage of this knowledge and allocates the destination memory upfront
and then simply writes directly into it. By doing this, we reduce heap pressure
and eliminate an intermediate copying operation.
This commit also renames the `s` parameter to something more descriptive.
`toLower` worked by using the GVPR state’s temporary buffer, `tmp`, to
incrementally construct a lowercase copy of the input. This works but is
suboptimal. When printing to the temporary buffer, it has no knowledge of what
we are up to and needs to make heuristic-based decisions on how to allocate
memory. But we know the exact size of the result string already. This commit
takes advantage of this knowledge and allocates the destination memory upfront
and then simply writes directly into it. By doing this, we reduce heap pressure
and eliminate an intermediate copying operation.
This commit also renames the `s` parameter to something more descriptive.
Roger Nesbitt [Wed, 23 Feb 2022 16:42:25 +0000 (05:42 +1300)]
Use "none" instead of "transparent" when painting in SVG
The SVG 1.1 spec, which SVGs generated by gvrender_core_svg.c are
labeled as, does not have the color "transparent" available which is
causing issues with SVG 1.1 parsers.
When specifying paint in the fill or stroke attributes, use "none"
instead of "transparent". When specifying a gradient color that has
zero opacity, use that color at zero opacity instead of "transparent".
When specifying a gradient with the graphviz transparent color, use
a zero-opacity black as this is how SVG interprets "transparent" in
gradients in SVG 2.0.
projects / graphviz / log