Obviously the number of errors that has occurred can never be negative. An
unsigned type more accurately indicates this. The only client of ingraphs that
actually checks the error count is nop, so this change has very little effect.
The exit status of nop was a bitwise-OR of two error counts. I do not see how
the actual value of this could ever be useful, except for being non-zero.
However, a non-zero exit status is not always an error. E.g. on VMS there are
some non-zero exit statuses that still indicate success.
This change causes nop to more reliably and portably exit with EXIT_FAILURE when
encountering errors.
The compiler could probably determine this already, but this makes it a little
more obvious that it can be inlined and the useString symbol can be the start of
the data rather than a pointer to the start of the data.
This case is never expected to be reachable. However, having a default case
helps catch when new options are introduced in the getopt call but not handled
in the switch. In a sense, this is a defensive coding measure. This also
squashes a -Wswitch-default compiler warning.
The generated config.h only contains a number of #defines, so its inclusion at
the end of a file has no effect. All code following this line was removed in 7cc812f5c1b64832a0324a293cc8968ba044e0a4 which should have also removed this
line but did not.
Ryan Schmidt [Sat, 8 May 2021 14:11:01 +0000 (14:11 +0000)]
Add --tag=CC when compiling quartz plugin
Edit by Matthew Fernandez: When compiling libraries, libtool infers a so called
“tag” indicating the language model it should use when creating objects.
The libtool docs¹ claim `CC` is inferred when no tag is explicitly given, but
empirically this does not seem to be true. On macOS, some versions of libtool
give up when seeing a .m file with the error:
libtool: compile: unable to infer tagged configuration
libtool: error: specify a tag with '--tag'
To fix this, we now explicitly set the `CC` tag. Note that this is not guarded
to be macOS-only because this action needs to be taken in all environments with
Objective-C support that are building the Quartz plugin (if this is in fact
possible on anything other than macOS).
This commit also fixes a further problem where AM_LIBTOOLFLAGS was misspelled
as AM_LIBTOOLSFLAGS. It looks like 79aa7d77ef433d66c0142168d2817b4c7014d263
previously tried to fix this same problem but (1) the WITH_DARWIN9 guard is
insufficient for recent macOS versions and (2) the typo made this whole change
a no-op.
This issue was reported by downstream Macports users.
Magnus Jacobsson [Tue, 25 May 2021 19:43:32 +0000 (21:43 +0200)]
Fix ASan odr_violation in mm2gv by not linking to libcommon
libcommon is a static lib, but is already part of the dynamic libgvc.
The error message was:
==73343==ERROR: AddressSanitizer: odr-violation (0x555555789760):
[1] size=8 'fdp_parms' /home/magjac/graphviz/lib/common/globals.c:30:20
[2] size=8 'fdp_parms' /home/magjac/graphviz/lib/common/globals.c:30:20
These globals were registered at these points:
[1]:
#0 0x7ffff760d938 in __asan_register_globals ../../../../src/libsanitizer/asan/asan_globals.cpp:341
#1 0x555555693361 in _sub_I_00099_1 (/home/magjac/build-graphviz-Desktop-Debug/cmd/tools/mm2gv+0x13f361)
#2 0x55555569376c in __libc_csu_init (/home/magjac/build-graphviz-Desktop-Debug/cmd/tools/mm2gv+0x13f76c)
[2]:
#0 0x7ffff760d938 in __asan_register_globals ../../../../src/libsanitizer/asan/asan_globals.cpp:341
#1 0x7ffff6f7de38 in _sub_I_00099_1 (/home/magjac/build-graphviz-Desktop-Debug/lib/gvc/libgvc.so.6+0x420e38)
#2 0x7ffff7fda96d (/lib64/ld-linux-x86-64.so.2+0x1196d)
==73343==HINT: if you don't care about these errors you may set ASAN_OPTIONS=detect_odr_violation=0
SUMMARY: AddressSanitizer: odr-violation: global 'fdp_parms' at /home/magjac/graphviz/lib/common/globals.c:30:20
==73343==ABORTING
commit generated ps_font_equiv.h and remove generator for this
The header ps_font_equiv.h was generated from 3 files, fontmap.cfg,
ps_font_equiv.txt, and ps_fontmap.txt. The generator itself was the Perl file,
mksvgfonts.pl.
The generation process is deterministic and not dependent on the end user’s
system. In fact, ps_font_equiv.h is generated during CI and included in the
portable source tarball. Furthermore the main driving source for this,
ps_font_equiv.txt is no clearer or more commented than the generated header.
Finally, a copy of the generated header was actually already committed to the
repository under windows/include/ for the MSBuild work flow that does not want
to call Perl.
For these reasons, having this as a generated file was no advantage. In fact,
this was a net negative in the build process as it is the only thing in the
build that requires Perl. As of this commit, Perl is no longer required to
build Graphviz. Perl is still necessary for building some optional components
like the Graphviz bindings for Perl.
consistently use float arithmetic in build_rotmatrix
By avoiding mixing doubles and floats, we avoid ever down-converting from a
float to a double and losing precision. Squashes some -Wfloat-conversion
warnings.
consistently use float arithmetic in tb_project_to_sphere
By avoiding mixing doubles and floats, we avoid ever down-converting from a
float to a double and losing precision. Squashes a -Wfloat-conversion warning.
rephrase some magic numbers in Smyrna into their originating computation
This removes the use of some double literals with implicit conversion to floats,
squashing some -Wfloat-conversion compiler warnings. It also makes the resulting
code clearer. The compiler constant-folds these at -O1 and above, so there is no
loss of performance. The resulting expressions are slightly different to the
original, but if anything they are a more accurate representation of the intent
here.
By avoiding mixing doubles and floats, we avoid ever down-converting from a
float to a double and losing precision. Squashes some -Wfloat-conversion
warnings.
By avoiding mixing doubles and floats, we avoid ever down-converting from a
float to a double and losing precision. Squashes a -Wfloat-conversion warning.
use more appropriate square root function in vlength
The square root functions sqrt and sqrtf compute on doubles and floats
respectively. Using the float version here avoids a lossy conversion from double
to float when returning from this function.
This function was mixing floats and doubles, leading to mixed precision issues.
Standardizing on float usage throughout leads to more predictable behavior and
removes a number of -Wfloat-conversion warnings.
This marks the class as unnecessary outside of its containing translation unit.
This further emphasizes that is is an implementation detail of the containing
file as well as allowing the compiler to aggressively optimize its layout.
manage Node::r as a reference instead of a pointer
Generally having class members that are references is an anti-pattern. However,
in this case the Node class is only used within generate-constraints.cpp with
all Node objects having limited lifetimes. It is essentially an implementation
detail of generate-constraints.cpp, irrelevant to the outside. Making its
Rectangle member a reference will ease some upcoming changes.
take const references in Rectangle::overlap{X|Y} instead of mutable pointers
These functions do not handle nullptr and do not modify their parameter, so a
const reference is more appropriate. This change will ease some future work
towards removing manual memory management.
Apart from being unused, this function has multiple issues:
1. Comment typo “separater.”
2. Comment discusses a path returning 1 that does not exist.
3. Useless return value.
4. The last store to len in the last loop is dead. This variable is unused
after this point because the containing loop is exited and it is unused in
the remainder of the function.
5. The two mallocs in the last loop over-allocate by one byte. The len
variable tracking the length of strings being constructed here was
incremented *past* the '\0' terminator in the previous line. Thus the
length of the string including '\0' is len, not len + 1.
6. The allocations in the loop are essentially doing an open-coded strdup (or
even more efficiently, strndup).
7. Both the large static buffer swork and the optional larger allocation into
swork1 are unnecessary. Tracking offset and length in s would allow using
the original memory as a source to any strndup/strncpy.
8. The ss allocation reserves one entry too many. The number of tokens is
ntokens, not ntokens + 1, and this array is not zeroed on allocation so the
last entry merely contains garbage.
projects / graphviz / log