scformat: use vmalloc instead of vmresize when processing '['
Though it is hard to see, the previous code bottomed out in a call to vmresize.
After allocating, it zeroes the allocated region. So vmresize (itself calling
realloc) signals an unnecessary constraint to the allocator that previous data
in this memory must be preserved.
To simplify this, this change converts the sequence to a vmfree of the prior
memory and then a new vmalloc, more clearly divorcing the new buffer from the
old one.
Buffers of length MAXNAME are printed into, including in a case where the
printed string is "(EXTERNAL:%d)". This needs a maximum of 23 bytes, not 16
bytes as was previously used. This overflow looks impossible to actually trigger
because I believe this code path is only used in the case of a bug in the lexer
itself. Hence no changelog entry for this.
This issue was exposed when moving sfsprintf calls to snsprintf, as the compiler
understands the semantics of the latter and knows how to warn about detectable
overflows. Related to #1998.
It is not clear to me what the purpose of this variable is/was. This Makefile
was added in adf9f9ad70662c0ca291feea1eb51df113c7d281. Even in that revision,
the variable was unused and refers to the non-existent file graph.tex.
None of the allocations in ccomps could tolerate failure. So this change makes
them all call wrappers that cleanly abort in the event of out-of-memory. Note
that this also fixes an issue where ccomps incorrectly identified itself as `gc`
in one of the previous failure messages.
The `undef` part of this macro juggling was typoed as `WIN32_STATIC` in commit b26b5fc076c1b5d9919ce79c807f5b3921149597, so it never properly undid the
preceding `inline` redirection. However, this redirection is unnecessary anyway.
Contrary to the Microsoft docs,¹ the `inline` keyword seems understood in *both*
C and C++.
Prior to this commit, the gdefs.h header was generated by a C program, mkdefs.c.
There were a number of issues with this approach:
1. The CMake build system was assuming the compiler to build mkdefs.c and the
compiler to build Graphviz itself were the same. This is not necessarily
true when cross-compiling.
2. Generation under MSBuild seems to have been impractical, so the generated
header was checked in to the repository under windows/include/gvpr/gdefs.h,
somewhat defeating the purpose of making it generated.
3. The CMake build system seems to not have been setup to correctly compile
mkdefs.c under all circumstances (see #2101).
This change removes any reliance on a host C compiler and instead uses a series
of X macros¹ to achieve the same effect. The values of all generated constants
and the content of generated structures is intended to be unchanged, though some
#defines have been altered to enums. In these cases, there was no advantage to
using a macro and multiple advantages to not using a macro.
This change is affecting a shipped header (gdefs.h) and also removes it from the
list of shipped headers. Installing it appears to have been a mistake as there
is no easy way for end users to use it. The header, fully expanded, still relies
on further expansion of macros that are only defined in expr.h, a header that is
not shipped.
lib/expr: use RSH as a constant for >> instead of RS
A couple of places in the code base are dodging an `RS` symbol apparently
introduced by an HPPA header. It is simpler to just use a symbol that does not
collide at all.
Chris Mansley [Wed, 21 Jul 2021 19:33:42 +0000 (19:33 +0000)]
Initialize nPasses in aspect_t struct in SetAspect
In dotLayout, nPasses is decremented in a while loop. Since it may be
uninitialized, this triggers clang's UndefinedBehaviorSanitizer:
signed-integer-overflow. While behavior does not change because the loop also
checks nextIter, which is initialized, this change fixes the sanitizer error.
rewrite edgepaint command line parsing using getopt_long
This change makes edgepaint command line parsing code more standard and robust.
It now rejects invalid arguments and takes more standard GNU-style double dash
prefixed options. Note that the old style single dash prefixed options are also
still accepted for compatibility purposes. Closes #1971.
When passing the option `-lightness` multiple times, pointers to previous
lightness strings would be overwritten and lost. This is unlikely to have had a
significant effect. Related to #1971.
add a test case for edgepaint command line parsing
This is a safe guard for upcoming changes that will rewrite how edgepaint parses
command line options. We want to ensure the existing interpretation of options
is not broken by this change. Related to #1971.
reflow edgepaint man page and make content consistent
Text is now wrapped at 80 columns (in the source, not the `man` output display),
sentences begin with capitals and end with periods, and `\fR` is used instead of
`\fP`. `\fR` sets regular text while `\fP` returns to the previous font. On the
surface, `\fP` sounds better, but troff only remembers the immediately prior
font, rather than a stack. So `\fP`s do not nest. With this in mind, `\fR` is
simpler and more comprehensible.
str_mpy: [nfc] pre-compute and allocate the result string
This change does not affect the functionality of this function, but it has two
motivating advantages:
1. The temporary scratch buffer `ex->tmp` is no longer used. Though it is not
obvious without auditing a lot of surrounding code, the data written into
this buffer does not need to be retained beyond the lifetime of this
function. Removing its use not only removes a code path through sfio, but
decouples this code from other code using `ex->tmp` making it easier to
understand. Related to #1873, #1998.
2. The prior code used an sfio temporary buffer to construct the result string
and then duplicated it into a vmalloc-allocated buffer. This is reasonable
as vmalloc has no support for incrementally constructing dynamically
allocated strings. However we can avoid the intermediate sfio buffer by
simply pre-computing the final vmalloc allocation that will be needed. This
change does exactly that and simply writes the result once into its final
destination instead of copying through an intermediate buffer. This
should not only (slightly) decrease transient heap pressure, but also
(again slightly) accelerate the performance of this function.
Both these effects are a simplification with respect to how the compiler sees
this function. That is, an optimizing compiler should now better comprehend the
intent of this function and be able to more aggressively specialize and inline
it where relevant.
Upcoming changes will improve the efficiency of this function and decrease its
coupling with other operations. Rather than introduce these new changes in a
differing style, this preparatory commit rewrites the existing functionality in
this style first, without affecting its behavior. Related to #1873, #1998.
str_mod: [nfc] pre-compute and allocate the result string
This change does not affect the functionality of this function, but it has two
motivating advantages:
1. The temporary scratch buffer `ex->tmp` is no longer used. Though it is not
obvious without auditing a lot of surrounding code, the data written into
this buffer does not need to be retained beyond the lifetime of this
function. Removing its use not only removes a code path through sfio, but
decouples this code from other code using `ex->tmp` making it easier to
understand. Related to #1873, #1998.
2. The prior code used an sfio temporary buffer to construct the result string
and then duplicated it into a vmalloc-allocated buffer. This is reasonable
as vmalloc has no support for incrementally constructing dynamically
allocated strings. However we can avoid the intermediate sfio buffer by
simply pre-computing the final vmalloc allocation that will be needed. This
change does exactly that and simply writes the result once into its final
destination instead of copying through an intermediate buffer. This
should not only (slightly) decrease transient heap pressure, but also
(again slightly) accelerate the performance of this function.
Both these effects are a simplification with respect to how the compiler sees
this function. That is, an optimizing compiler should now better comprehend the
intent of this function and be able to more aggressively specialize and inline
it where relevant.
Upcoming changes will improve the efficiency of this function and decrease its
coupling with other operations. Rather than introduce these new changes in a
differing style, this preparatory commit rewrites the existing functionality in
this style first, without affecting its behavior. Related to #1873, #1998.
str_xor: [nfc] pre-compute and allocate the result string
This change does not affect the functionality of this function, but it has two
motivating advantages:
1. The temporary scratch buffer `ex->tmp` is no longer used. Though it is not
obvious without auditing a lot of surrounding code, the data written into
this buffer does not need to be retained beyond the lifetime of this
function. Removing its use not only removes a code path through sfio, but
decouples this code from other code using `ex->tmp` making it easier to
understand. Related to #1873, #1998.
2. The prior code used an sfio temporary buffer to construct the result string
and then duplicated it into a vmalloc-allocated buffer. This is reasonable
as vmalloc has no support for incrementally constructing dynamically
allocated strings. However we can avoid the intermediate sfio buffer by
simply pre-computing the final vmalloc allocation that will be needed. This
change does exactly that and simply writes the result once into its final
destination instead of copying through an intermediate buffer. This
should not only (slightly) decrease transient heap pressure, but also
(again slightly) accelerate the performance of this function.
Both these effects are a simplification with respect to how the compiler sees
this function. That is, an optimizing compiler should now better comprehend the
intent of this function and be able to more aggressively specialize and inline
it where relevant.
Upcoming changes will improve the efficiency of this function and decrease its
coupling with other operations. Rather than introduce these new changes in a
differing style, this preparatory commit rewrites the existing functionality in
this style first, without affecting its behavior. Related to #1873, #1998.
str_and: [nfc] pre-compute and allocate the result string
This change does not affect the functionality of this function, but it has two
motivating advantages:
1. The temporary scratch buffer `ex->tmp` is no longer used. Though it is not
obvious without auditing a lot of surrounding code, the data written into
this buffer does not need to be retained beyond the lifetime of this
function. Removing its use not only removes a code path through sfio, but
decouples this code from other code using `ex->tmp` making it easier to
understand. Related to #1873, #1998.
2. The prior code used an sfio temporary buffer to construct the result string
and then duplicated it into a vmalloc-allocated buffer. This is reasonable
as vmalloc has no support for incrementally constructing dynamically
allocated strings. However we can avoid the intermediate sfio buffer by
simply pre-computing the final vmalloc allocation that will be needed. This
change does exactly that and simply writes the result once into its final
destination instead of copying through an intermediate buffer. This
should not only (slightly) decrease transient heap pressure, but also
(again slightly) accelerate the performance of this function.
Both these effects are a simplification with respect to how the compiler sees
this function. That is, an optimizing compiler should now better comprehend the
intent of this function and be able to more aggressively specialize and inline
it where relevant.
Upcoming changes will improve the efficiency of this function and decrease its
coupling with other operations. Rather than introduce these new changes in a
differing style, this preparatory commit rewrites the existing functionality in
this style first, without affecting its behavior. Related to #1873, #1998.
str_ior: [nfc] pre-compute and allocate the result string
This change does not affect the functionality of this function, but it has two
motivating advantages:
1. The temporary scratch buffer `ex->tmp` is no longer used. Though it is not
obvious without auditing a lot of surrounding code, the data written into
this buffer does not need to be retained beyond the lifetime of this
function. Removing its use not only removes a code path through sfio, but
decouples this code from other code using `ex->tmp` making it easier to
understand. Related to #1873, #1998.
2. The prior code used an sfio temporary buffer to construct the result string
and then duplicated it into a vmalloc-allocated buffer. This is reasonable
as vmalloc has no support for incrementally constructing dynamically
allocated strings. However we can avoid the intermediate sfio buffer by
simply pre-computing the final vmalloc allocation that will be needed. This
change does exactly that and simply writes the result once into its final
destination instead of copying through an intermediate buffer. This
should not only (slightly) decrease transient heap pressure, but also
(again slightly) accelerate the performance of this function.
Both these effects are a simplification with respect to how the compiler sees
this function. That is, an optimizing compiler should now better comprehend the
intent of this function and be able to more aggressively specialize and inline
it where relevant.
Upcoming changes will improve the efficiency of this function and decrease its
coupling with other operations. Rather than introduce these new changes in a
differing style, this preparatory commit rewrites the existing functionality in
this style first, without affecting its behavior. Related to #1873, #1998.
projects / graphviz / log