remove 'pointfof' and replace with aggregate initialization
Now that Graphviz is compiled with C99, there does not seem to be much advantage
in retaining this helper function. We have initialization syntax that is the
same number of characters and can be understood locally without having to lookup
the definition of `pointfof`.
unconditionally use 'deflateBound' when zlib is enabled
When Graphviz was built with support for zlib-based compression, it would only
use `deflateBound` if it was discovered at build time. The `deflateBound` code
path provides a tighter estimate of final compressed size, allowing compression
to minimize its intermediate heap allocations.
The CMake build system did not attempt to discover the presence of
`deflateBound` (config-cmake.in). The Windows build hard coded `deflateBound` as
unavailable (windows/include/config.h). So both these builds would result in a
Graphviz that would use more memory than necessary when writing compressed
output formats.
`deflateBound` arrived in zlib 1.2.0, released on 2003-03-09. It seems safe to
unconditionally assume its existence in 2022. The estimate `deflateBound`
provides has improved in successive releases, with even the latest zlib 1.2.13
released on 2022-10-13 further tightening its estimate. So it is expected that
Graphviz users, with this change, will get lower memory usage during writing
compressed formats, and then lower still as newer zlib releases propagate
through the ecosystem.
Calls to this function are generated by Flex – it normally generates calls to
`yyerror` but we alter the Flex prefix to `aag` – but it does not generate a
prototype for the function. So squash the compiler’s overly cautious warning:
../../lib/cgraph/scan.l: At top level:
../../lib/cgraph/scan.l:219:6: warning: no previous prototype for ‘aagerror’
[-Wmissing-prototypes]
219 | void aagerror(const char *str)
| ^~~~~~~~
../../lib/cgraph/scan.l: In function ‘aagerror’:
../../lib/cgraph/scan.l:232:14: warning: switch missing default case
[-Wswitch-default]
232 | else switch (YYSTATE) {
| ^~~~~~
This switch is adding more information to the error message based on which
lexing state we are in, of which four are defined: `INITIAL` (the built-in start
state), `comment`, `hstring`, and `qstring`. In the `INITIAL` state there is no
extra information to add.
cgraph storeFileName: squash warnings when dealing with buffer lengths
This removes the following compiler warnings which were particularly problematic
as they were pointing to the wrong source lines. It is unclear whether this is
due to a Flex bug or the way we are calling Flex.
../../lib/cgraph/scan.l: In function ‘storeFileName’:
../../lib/cgraph/scan.l:98:28: warning: conversion to ‘size_t’ {aka ‘long
unsigned int’} from ‘int’ may change the sign of the result
[-Wsign-conversion]
98 | static char* buf;
| ^
../../lib/cgraph/scan.l:98:37: warning: conversion to ‘size_t’ {aka ‘long
unsigned int’} from ‘int’ may change the sign of the result
[-Wsign-conversion]
98 | static char* buf;
| ^
../../lib/cgraph/scan.l: In function ‘ppDirective’:
../../lib/cgraph/scan.l:125:29: warning: conversion from ‘long int’ to ‘int’
may change value [-Wconversion]
125 | while (*e && *e != '"') e++;
| ~^~
greg [Fri, 7 Oct 2022 00:34:47 +0000 (17:34 -0700)]
Increase stack size for dot MSBUILD project file (avoid crash on larger graphs)
Comment from Matthew Fernandez: It is believed this resolves some instances of
stack overflow in `dfs_enter_outedge` experienced with larger graphs on Windows.
Co-authored-by: Matthew Fernandez <matthew.fernandez@gmail.com>
Return value
…
The currently selected filename, or NULL if no file is selected, or the
selected file can’t be represented with a local filename. Free with g_free().
The caller of the method takes ownership of the data, and is responsible for
freeing it.
Contrary to this, `savefiledlg` was duplicating the pointed to data and then
losing the pointer `gtk_file_chooser_get_filename` returned.
The straightforward fix to this would be to retain the pointer and then `g_free`
it after copying its contents into the buffer `xbuf`. However, we can instead
refactor this function to avoid the copy altogether and simply pass the original
memory back to the caller, making this both a fix and an optimization.
On top of this, `on_gvprbuttonsave_clicked` was never calling `agxbfree` on its
local `xbuf`. By moving to a standard string, we also fix this memory leak.
Return value
…
The currently selected filename, or NULL if no file is selected, or the
selected file can’t be represented with a local filename. Free with g_free().
The caller of the method takes ownership of the data, and is responsible for
freeing it.
Contrary to this, `openfiledlg` was duplicating the pointed to data and then
losing the pointer `gtk_file_chooser_get_filename` returned.
The straightforward fix to this would be to retain the pointer and then `g_free`
it after copying its contents into the buffer `xbuf`. However, we can instead
refactor this function to avoid the copy altogether and simply pass the original
memory back to the caller, making this both a fix and an optimization.
graphml2gv: remove 'composite_buffer' from 'userdata_t'
1d28d7d2b4d2b2551bd1f432aa175f54a69364a4 seems to have copied this
implementation of `userdata_t` from cmd/tools/gxl2gv.c without noticing the
`composite_buffer` member is never used in graphml2gv.
graphml2gv: remove 'xml_attr_value' from 'userdata_t'
1d28d7d2b4d2b2551bd1f432aa175f54a69364a4 seems to have copied this
implementation of `userdata_t` from cmd/tools/gxl2gv.c without noticing the
`xml_attr_value` member is never written to in graphml2gv.
graphml2gv: remove 'xml_attr_name' from 'userdata_t'
1d28d7d2b4d2b2551bd1f432aa175f54a69364a4 seems to have copied this
implementation of `userdata_t` from cmd/tools/gxl2gv.c without noticing the
`xml_attr_name` member is never written to in graphml2gv.
This allows dropping several workarounds in the CMake build system. The only
supported platform with a CMake version less than 3.13.0 is Ubuntu 18.04. Given
CMake is not the default build system, the age of Ubuntu 18.04 makes it unlikely
there are users there building Graphviz from source, and later CMake versions
are easily obtainable from Kitware, it seems reasonable to increase this
requirement.
The `rsvg_term` function has been a no-op for some time:¹
rsvg_term has been deprecated since version 2.36 and should not be used in
newly-written code.
There is no need to de-initialize librsvg.
This function does nothing.
Graphviz was calling it when linked against librsvg 2.36 (2.36.0 was released
2012-03-26). I do not know why this code was conditional as `rsvg_term` has been
a no-op since 2.35.0.² Moreover, calling it prior to it becoming a no-op was
wrong too. The commit that removed its functionality notes:
rsvg_term() was dangerous to call
The problem is that it previously called libxml2’s `xmlCleanupParser` whose
documentation states:³
This function name is somewhat misleading. It does not clean up parser state,
it cleans up memory allocated by the library itself. It is a cleanup function
for the XML library. It tries to reclaim all related global memory allocated
for the library processing. It doesn't deallocate any document related memory.
One should call xmlCleanupParser() only when the process has finished using
the library and all XML/HTML documents built with it. See also xmlInitParser()
which has the opposite function of preparing the library for operations.
WARNING: if your application is multithreaded or has plugin support calling
this may crash the application if another thread or a plugin is still using
libxml2. It's sometimes very hard to guess if libxml2 is in use in the
application, some libraries or plugins may use it without notice. In case of
doubt abstain from calling this function or do it just before calling exit()
to avoid leak reports from valgrind !
This seems to have come to light ~2010.⁴ But presumably there is still a long
tail of software in the wild still, like Graphviz, erroneously calling
`rsvg_term`.
The `-ffast-math` compiler option enables a mode of floating point semantics
that are heavily relaxed from strict IEEE 754 conformance in the name of speed.¹
The permissible optimizations under `-ffast-math` include transformations that
produce floating point results most users would consider wrong. It is unclear
why “do math fast but incorrectly” is a useful mode. It seems the world is
finally coming around to this tenet.²
This change stops enabling `-ffast-math` anywhere in the Autotools build system.
Gitlab: fixes #1412, #2296 Reported-by: Kasper Daniel Hansen Reported-by: Ross Barnowski
¹ https://gcc.gnu.org/wiki/FloatingPointMath
² https://simonbyrne.github.io/notes/fastmath/,
https://moyix.blogspot.com/2022/09/someones-been-messing-with-my-subnormals.html
remove suppression of -ffast-math when building with ICC
The Intel C Compiler does not support the `-ffast-math` command line option, so
this logic was suppressing it. But it is coming before anything in the Graphviz
build system itself that would add `-ffast-math`. So it is actually suppressing
a `-ffast-math` addition coming from the user’s environment. If you are a user
compiling Graphviz with ICC, I have a very simple alternative for you: do not
pass `-ffast-math` explicitly.
projects / graphviz / log