expr: remove unused parameters from 'Exdisc_t.binaryf'
I guess when I committed b0ec7b2eb448a2cb68ffb3751e9e054aecc60c24 it was not
obvious to me that this is entirely part of private internal interfaces and the
callback signature involved is not exposed publicly.
expr: remove unused parameters from 'Exdisc_t.convertf'
I guess when I committed 17479ab6569d40a4778870f712226aa7916f3ca3 it was not
obvious to me that this is entirely part of private internal interfaces and the
callback signature involved is not exposed publicly.
expr: remove further unused parameters from 'Exdisc_t.reff'
I guess when I committed ef3ed2a98a46d0641d936c62fde78e719cef98f8 it was not
obvious to me that this is entirely part of private internal interfaces and the
callback signature involved is not exposed publicly.
expr: replace 'exdump' SFIO buffer with an 'agxbuf'
Unfortunately there is no easy way to stage this. We need to do it all at once,
updating every use of `deparse` and all `exdump`’s internals.
This change improves locality – it is more obvious to both users and the
compiler that the contents of this temporary buffer does not need to be retained
beyond calls to `exerror`. This is a small step towards deprecating SFIO.
Now that `Excc_t` is encapsulated in excc.c and no longer considered an
extensible struct, we can stop playing macro tricks to give it extra fields and
just express these more naturally.
This function invokes `dtwalk` passing the callback `global` to be called with
a state parameter it supplies. It was supplying `cc`, but the callback only uses
a single member of this struct. So we can simplify this code by just passing the
stream the callback writes to, `cc->ccdisc-text`, or equivalently `disc->text`.
ccomps: remove 'N_NEW' that was only used in one place
This brings us one step closer to having a single `N_NEW` macro in the tree and
not having to constantly wonder which of the `N_NEW` definitions is in scope at
any point where you see it called.
Rather than allocating this upfront and then potentially needing to free it
later, we can use a string view to postpone the allocation to when we know it is
actually needed. This works because the lifetime of the backing memory extends
until the call to `g_free(families)`. This also fixes the lack of allocation
failure checks; instead of `strdup` we now call `strview_str` that gracefully
exits on allocation failure.
The type of the fields of `pts` can vary between `float` and `double` depending
compile time options, which is what this code was attempting to deal with. But
`float` and `double` are treated identically with respect to `printf`, so there
is no need to cast.
Here again the proper solution would be to make the struct’s field also a
`size_t` but once again it is part of the public API that we would like to avoid
breaking.
pathplan Pobsopen: use a 'size_t' when counting objects
This squashes 3 -Wsign-conversion warnings and is generally closer to what we
would like to do here. The “proper” fix is for fields like `vconfig_t.N` to
become `size_t` instead of `int`. But unfortunately they are part of the public
API, and it seems undesirable to break API for this.
Note that an assumption previously implicit in this function, that all inputs
had a non-negative polygon count, is now an explicit assertion.
The calling code does not rely on the semantics of `malloc` returning `NULL`
when called with a 0 size. It unconditionally frees these arrays in `Pobsclose`
so allocations returning a non-`NULL` pointer for 0-sized allocations (which
Glibc typically does) is fine. Maintaining a wrapper that enforces this is
unnecessary.
This also removes the `method` parameter to this function, which was only ever
set to a single value. This squashes 8 compiler warnings and drops a lot of dead
code.
gvpr canontoken: use char instead of unsigned char, introduce casts
It is not clear to me why this code was using unsigned char when it can do the
same thing with less typing and fewer compiler warnings using char. This also
introduces casts to squash warnings from some of the more pedantic compiler
implementations. See 6c29170f9f29466374fbc6e8e62a1b6916c6bc59 for details.
ast onematch: rephrase hash-based switch into string comparisons
This code was using a hand rolled hash function to implement a series of string
comparisons as a jump table. I guess at some point this must have been a
necessary optimization due to limitations of the day’s compilers/machines. In a
modern environment, this is a deoptimization, impeding the compiler’s ability to
understand your intent. Modern compilers know the string comparison functions as
built-ins and can use SIMD¹/SWAR² tricks to emit a short string comparison as a
single instruction. They are also capable of transforming an if-then-else ladder
into a switch if their heuristics predict it will be worthwhile.
The computation of the return value for this function was relying on string
lengths that fitted in an `int`, something that is generally but not always
true. The compiler complained about this with -Wconversion. The only caller of
this function does not use the return value, so lets just remove it.
This is computing the number of bytes remaining in `buf`; the number of bytes
between a pointer to the current offset and the end of `buf`. Thus it is always
non-negative.
projects / graphviz / log