Pierre Labastie [Tue, 15 Nov 2022 08:03:54 +0000 (09:03 +0100)]
Fix php install for SWIG-4.0.1
This version of SWIG does not generate gv.php anymore, so
installing it shouldn't be tried. The autotools framework is
already there, so just use a conditional in Makefile.am.
The test job was doing this but the build job was not. Technically I think this
is unnecessary (I think CMake includes `CFLAGS` or `CXXFLAGS` when calling `ld`)
but it seems worth being consistent.
CI: make UBSan errors in compiled test cases fail the test
0d0c20797a0cf2937886945f3aecc4e8a9a55e8c adjusted this for the build job, but
not for the test job. It is useful to have this enabled for the test job as well
because some test cases involve compiling C code, for which we want full ASan
and UBSan enabled too.
shortest.c: In function ‘growtris’:
shortest.c:518:39: warning: conversion to ‘long unsigned int’ from ‘int’ may
change the sign of the result [-Wsign-conversion]
518 | tris = realloc(tris, TRIANGLESIZE * newtrin);
| ^
At the call site for `growtris`, it is not clear to me why `trin + 20` is
enough. That is, it seems like this branch can be hit, reallocation can succeed,
but `tril` was greater than `trin + 20` so the array remains too small for the
follow on computation. But this is a separate problem.
pathplan: use a 'size_t' for counting allocated points
Squashes:
shortest.c:500:41: warning: conversion to ‘long unsigned int’ from ‘int’ may
change the sign of the result [-Wsign-conversion]
500 | pnls = realloc(pnls, POINTNLINKSIZE * newpnln);
| ^
shortest.c:505:44: warning: conversion to ‘long unsigned int’ from ‘int’ may
change the sign of the result [-Wsign-conversion]
505 | pnlps = realloc(pnlps, POINTNLINKPSIZE * newpnln);
| ^
It would be nice to adjust `Ppoly_t.pn` to avoid the casting, but it is part of
the public API and it does not seem worth an API break for this.
pathplan: phrase comparison in 'splinefits' with a tolerance
From the adjustments to `a` in this function, it can be seen this is equivalent.
This squashes the warning:
route.c: In function ‘splinefits’:
route.c:252:15: warning: comparing floating-point with ‘==’ or ‘!=’ is unsafe
[-Wfloat-equal]
252 | if (a == 0) {
| ^~
This squashes the following warnings on CentOS 7 where the compiler believes
the sequence ID may overflow:
node.c: In function 'agnodebefore':
node.c:370:23: warning: conversion to 'unsigned int:28' from 'int' may alter
its value [-Wconversion]
AGSEQ(n) = AGSEQ(n) + 1;
^
node.c:376:26: warning: conversion to 'unsigned int:28' from 'int' may alter
its value [-Wconversion]
AGSEQ(snd) = AGSEQ(fst) - 1;
^
This attempts to replicate how this target is defined in the Autotools build
system, but involved some amount of guesswork because the Autotools build system
is not explicit with _why_ it is adding particular dependencies to link/include
lists.
The rpath tweak in this commit required surprisingly lengthy debugging to arrive
at. I am still not completely confident it is correct nor that this is the right
way to do such things.
dot_builtins: use 'dllimport' even when LTDL is in use
I am not sure why 53eec6fb96646da693e5e8f78047cccf91774dbb included LTDL in the
conditional here. Whether LTDL is in use or not has no bearing on whether we
`dllimport` mark the plugin libraries, because we are unconditionally
dynamically linking them (as opposed to LTDL-based on demand loading).
CMake: fix: add Zlib include directories when building with Zlib
This worked out before because everywhere we test has Zlib installed in the
default system paths. But technically this should be setup in order to correctly
use a Zlib installed somewhere else.
This code is trying to abbreviate printing `0.000` or `1.000`. We can more
accurately describe the situations under which this will happen, squashing:
gvrender_core_svg.c: In function ‘svg_print_stop’:
gvrender_core_svg.c:525:16: warning: comparing floating-point with ‘==’ or
‘!=’ is unsafe [-Wfloat-equal]
525 | if (offset == 0.0)
| ^~
gvrender_core_svg.c:527:21: warning: comparing floating-point with ‘==’ or
‘!=’ is unsafe [-Wfloat-equal]
527 | else if (offset == 1.0)
| ^~
`gvprintdouble` prints to two decimal places. So we can take this into account
when deciding whether we need to print a non-default pen width, squashing:
gvrender_core_svg.c: In function ‘svg_grstyle’:
gvrender_core_svg.c:191:23: warning: comparing floating-point with ‘==’ or
‘!=’ is unsafe [-Wfloat-equal]
191 | if (obj->penwidth != PENWIDTH_NORMAL) {
| ^~
465cef702370966f17d27d455093cf69a651af9c fixed an issue where `id` attributes
were not propagated to SVG output. However unfortunately it broke references to
such attributes in fill gradients. This commit makes the corresponding
adjustment to the references, so they once again align with their targets.
I think this code originally intended to call `Qt::flush` but changes in Qt now
make ADL pick the wrong overload. This squashes the warnings below. We cannot
easily switch to the alternative the deprecation warning suggests because it is
unavailable in older versions of Qt.
main.cpp: In function ‘char** parseArgs(int, char**)’:
main.cpp:72:42: warning:
‘QTextStream& QTextStreamFunctions::flush(QTextStream&)’ is deprecated: Use
Qt::flush [-Wdeprecated-declarations]
72 | " unrecognized\n" << flush;
| ^~~~~
In file included from /usr/include/x86_64-linux-gnu/qt5/QtCore/QTextStream:1,
from mainwindow.h:19,
from main.cpp:24:
/usr/include/x86_64-linux-gnu/qt5/QtCore/qtextstream.h:294:76: note: declared
here
294 | Q_CORE_EXPORT QT_DEPRECATED_VERSION_X(5, 15, "Use Qt::flush")
QTextStream &flush(QTextStream &s);
|
main.cpp:72:42: warning:
‘QTextStream& QTextStreamFunctions::flush(QTextStream&)’ is deprecated: Use
Qt::flush [-Wdeprecated-declarations]
72 | " unrecognized\n" << flush;
| ^~~~~
In file included from /usr/include/x86_64-linux-gnu/qt5/QtCore/QTextStream:1,
from mainwindow.h:19,
from main.cpp:24:
/usr/include/x86_64-linux-gnu/qt5/QtCore/qtextstream.h:294:76: note: declared
here
294 | Q_CORE_EXPORT QT_DEPRECATED_VERSION_X(5, 15, "Use Qt::flush")
QTextStream &flush(QTextStream &s);
|
csettings.cpp: In function ‘bool loadAttrs(QString, QComboBox*, QComboBox*,
QComboBox*)’:
csettings.cpp:94:35: warning:
‘QTextStream& QTextStreamFunctions::flush(QTextStream&)’ is deprecated: Use
Qt::flush [-Wdeprecated-declarations]
94 | "\" for reading\n" << flush;
| ^~~~~
In file included from qt5/QtCore/qdebug.h:49,
from qt5/QtCore/qcborcommon.h:45,
from qt5/QtCore/qcborvalue.h:45,
from qt5/QtCore/qcborarray.h:43,
from qt5/QtCore/QtCore:38,
from qt5/QtWidgets/QtWidgetsDepends:3,
from qt5/QtWidgets/QtWidgets:3,
from csettings.cpp:16:
/usr/include/x86_64-linux-gnu/qt5/QtCore/qtextstream.h:294:76: note: declared
here
294 | Q_CORE_EXPORT QT_DEPRECATED_VERSION_X(5, 15, "Use Qt::flush")
QTextStream &flush(QTextStream &s);
|
csettings.cpp:94:35: warning:
‘QTextStream& QTextStreamFunctions::flush(QTextStream&)’ is deprecated: Use
Qt::flush [-Wdeprecated-declarations]
94 | "\" for reading\n" << flush;
| ^~~~~
In file included from qt5/QtCore/qdebug.h:49,
from qt5/QtCore/qcborcommon.h:45,
from qt5/QtCore/qcborvalue.h:45,
from qt5/QtCore/qcborarray.h:43,
from qt5/QtCore/QtCore:38,
from qt5/QtWidgets/QtWidgetsDepends:3,
from qt5/QtWidgets/QtWidgets:3,
from csettings.cpp:16:
/usr/include/x86_64-linux-gnu/qt5/QtCore/qtextstream.h:294:76: note: declared
here
294 | Q_CORE_EXPORT QT_DEPRECATED_VERSION_X(5, 15, "Use Qt::flush")
QTextStream &flush(QTextStream &s);
|
Similar to the previous commit, these are dealt with as `int` internally, so
better to use `int` the whole way through. Note that this rejects values that do
not fit in an `int` which would silently overflow before. This squashes:
gvusershape.c: In function ‘webp_size’:
gvusershape.c:301:21: warning: conversion to ‘int’ from ‘unsigned int’ may
change the sign of the result [-Wsign-conversion]
301 | us->w = w;
| ^
gvusershape.c:302:21: warning: conversion to ‘int’ from ‘unsigned int’ may
change the sign of the result [-Wsign-conversion]
302 | us->h = h;
| ^
gvusershape.c:308:21: warning: conversion to ‘int’ from ‘unsigned int’ may
change the sign of the result [-Wsign-conversion]
308 | us->w = w;
| ^
gvusershape.c:309:21: warning: conversion to ‘int’ from ‘unsigned int’ may
change the sign of the result [-Wsign-conversion]
309 | us->h = h;
| ^
gvusershape.c: In function ‘gif_size’:
gvusershape.c:321:17: warning: conversion to ‘int’ from ‘unsigned int’ may
change the sign of the result [-Wsign-conversion]
321 | us->w = w;
| ^
gvusershape.c:322:17: warning: conversion to ‘int’ from ‘unsigned int’ may
change the sign of the result [-Wsign-conversion]
322 | us->h = h;
| ^
gvusershape.c: In function ‘bmp_size’:
gvusershape.c:335:17: warning: conversion to ‘int’ from ‘unsigned int’ may
change the sign of the result [-Wsign-conversion]
335 | us->w = size_x_msw << 16 | size_x_lsw;
| ^~~~~~~~~~
gvusershape.c:336:17: warning: conversion to ‘int’ from ‘unsigned int’ may
change the sign of the result [-Wsign-conversion]
336 | us->h = size_y_msw << 16 | size_y_lsw;
| ^~~~~~~~~~
These are dealt with as `int` internally, so better to use `int` the whole way
through. Note that this rejects values that do not fit in an `int` which would
silently overflow before. This squashes:
gvusershape.c: In function ‘png_size’:
gvusershape.c:271:17: warning: conversion to ‘int’ from ‘unsigned int’ may
change the sign of the result [-Wsign-conversion]
271 | us->w = w;
| ^
gvusershape.c:272:17: warning: conversion to ‘int’ from ‘unsigned int’ may
change the sign of the result [-Wsign-conversion]
272 | us->h = h;
| ^
gvusershape.c: In function ‘ico_size’:
gvusershape.c:283:17: warning: conversion to ‘int’ from ‘unsigned int’ may
change the sign of the result [-Wsign-conversion]
283 | us->w = w;
| ^
gvusershape.c:284:17: warning: conversion to ‘int’ from ‘unsigned int’ may
change the sign of the result [-Wsign-conversion]
284 | us->h = h;
| ^
gvusershape.c: In function ‘jpeg_size’:
gvusershape.c:381:25: warning: conversion to ‘int’ from ‘unsigned int’ may
change the sign of the result [-Wsign-conversion]
381 | us->h = size_x;
| ^~~~~~
gvusershape.c:382:25: warning: conversion to ‘int’ from ‘unsigned int’ may
change the sign of the result [-Wsign-conversion]
382 | us->w = size_y;
| ^~~~~~
gvusershape.c:396:25: warning: conversion to ‘int’ from ‘unsigned int’ may
change the sign of the result [-Wsign-conversion]
396 | us->h = size_x;
| ^~~~~~
gvusershape.c:397:25: warning: conversion to ‘int’ from ‘unsigned int’ may
change the sign of the result [-Wsign-conversion]
397 | us->w = size_y;
| ^~~~~~
These values are eventually stored in `int` fields, so going through
`unsigned int` is unproductive. This squashes a number of warnings:
In file included from ../../lib/common/geom.h:19,
from ../../lib/common/types.h:37,
from gvusershape.c:28:
gvusershape.c: In function ‘svg_units_convert’:
../../lib/common/arith.h:59:46: warning: conversion to ‘unsigned int’ from
‘int’ may change the sign of the result [-Wsign-conversion]
59 | #define ROUND(f) ((f>=0)?(int)(f + .5):(int)(f - .5))
| ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
gvusershape.c:149:16: note: in expansion of macro ‘ROUND’
149 | return ROUND(n * POINTS_PER_INCH);
| ^~~~~
../../lib/common/arith.h:59:46: warning: conversion to ‘unsigned int’ from
‘int’ may change the sign of the result [-Wsign-conversion]
59 | #define ROUND(f) ((f>=0)?(int)(f + .5):(int)(f - .5))
| ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
gvusershape.c:151:16: note: in expansion of macro ‘ROUND’
151 | return ROUND(n * POINTS_PER_INCH / 96);
| ^~~~~
../../lib/common/arith.h:59:46: warning: conversion to ‘unsigned int’ from
‘int’ may change the sign of the result [-Wsign-conversion]
59 | #define ROUND(f) ((f>=0)?(int)(f + .5):(int)(f - .5))
| ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
gvusershape.c:153:16: note: in expansion of macro ‘ROUND’
153 | return ROUND(n * POINTS_PER_INCH / 6);
| ^~~~~
../../lib/common/arith.h:59:46: warning: conversion to ‘unsigned int’ from
‘int’ may change the sign of the result [-Wsign-conversion]
59 | #define ROUND(f) ((f>=0)?(int)(f + .5):(int)(f - .5))
| ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
gvusershape.c:155:16: note: in expansion of macro ‘ROUND’
155 | return ROUND(n);
| ^~~~~
../../lib/common/arith.h:59:46: warning: conversion to ‘unsigned int’ from
‘int’ may change the sign of the result [-Wsign-conversion]
59 | #define ROUND(f) ((f>=0)?(int)(f + .5):(int)(f - .5))
| ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
gvusershape.c:157:16: note: in expansion of macro ‘ROUND’
157 | return ROUND(n * POINTS_PER_CM);
| ^~~~~
../../lib/common/arith.h:59:46: warning: conversion to ‘unsigned int’ from
‘int’ may change the sign of the result [-Wsign-conversion]
59 | #define ROUND(f) ((f>=0)?(int)(f + .5):(int)(f - .5))
| ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
gvusershape.c:159:16: note: in expansion of macro ‘ROUND’
159 | return ROUND(n * POINTS_PER_MM);
| ^~~~~
gvusershape.c:261:13: warning: conversion to ‘int’ from ‘unsigned int’ may
change the sign of the result [-Wsign-conversion]
261 | us->w = w;
| ^
gvusershape.c:262:13: warning: conversion to ‘int’ from ‘unsigned int’ may
change the sign of the result [-Wsign-conversion]
262 | us->h = h;
| ^
GD plugin: use more appropriate type for points counting
Squashes the compiler warning:
gvrender_gd.c: In function 'gdgen_polygon':
gvrender_gd.c:491:6: warning: conversion to 'long unsigned int' from 'int'
may change the sign of the result [-Wsign-conversion]
points = realloc(points, n * sizeof(gdPoint));
^
gvloadimage_core.c: In function ‘core_loadimage_vml’:
gvloadimage_core.c:251:18: warning: conversion to ‘unsigned int’ from ‘int’
may change the sign of the result [-Wsign-conversion]
251 | graphHeight =(int)(job->bb.UR.y - job->bb.LL.y);
| ^
It is not clear why these were floats as they were operated on as ints. This
squashes the compiler warning:
gvrender_core_pov.c: In function ‘pov_begin_layer’:
gvrender_core_pov.c:459:18: warning: conversion from ‘int’ to ‘float’ may
change value [-Wconversion]
459 | layerz = layerNum * -10;
| ^~~~~~~~
gvc: squash 'api_t' related -Wsign-compare warnings
The range of either 'int' or 'size_t' is enough to cover all valid values this
takes on, so we can safely squash the following:
gvplugin.c: In function ‘gvPluginList’:
gvplugin.c:408:23: warning: comparison of integer expressions of different
signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
408 | for (api = 0; api < ARRAY_SIZE(api_names); api++) {
| ^
gvplugin.c: In function ‘gvplugin_write_status’:
gvplugin.c:450:23: warning: comparison of integer expressions of different
signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
450 | for (api = 0; api < ARRAY_SIZE(api_names); api++) {
| ^
gvplugin.c: In function ‘gvplugin_graph’:
gvplugin.c:501:27: warning: comparison of integer expressions of different
signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
501 | for (api = 0; api < ARRAY_SIZE(api_names); api++) {
| ^
gvplugin.c:680:27: warning: comparison of integer expressions of different
signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
680 | for (api = 0; api < ARRAY_SIZE(api_names); api++) {
| ^
gvc: squash -Wconversion warnings in 'gvevent_button_press'
Squashes the following compiler warnings. We know these are false positives due
to the switch cases in which this code appears.
gvevent.c:373:23: warning: conversion from ‘int’ to ‘unsigned char’ may change
value [-Wconversion]
373 | job->button = button;
| ^~~~~~
gvevent.c:378:23: warning: conversion from ‘int’ to ‘unsigned char’ may change
value [-Wconversion]
378 | job->button = button;
| ^~~~~~
gvevent.c:384:23: warning: conversion from ‘int’ to ‘unsigned char’ may change
value [-Wconversion]
384 | job->button = button;
| ^~~~~~
cgraph: fix mismatched type signatures between 'agobjfn_t' and 'agraphattr_init'
Squashes a compiler warning:
attr.c: In function ‘init_all_attrs’:
attr.c:534:37: warning: cast between incompatible function types from ‘void
(*)(Agraph_t *)’ {aka ‘void (*)(struct Agraph_s *)’} to ‘void (*)(Agraph_t
*, Agobj_t *, void *)’ {aka ‘void (*)(struct Agraph_s *, struct Agobj_s *,
void *)’} [-Wcast-function-type]
534 | agapply(root, (Agobj_t *) root, (agobjfn_t) agraphattr_init,
| ^
This code was incorrect. But on the most common architecture in contemporary
use, x86-64, the calling convention for both is fully in registers, so no user
visible misbehavior would have been seen.
cgraph: fix: align 'dict_relabel' with 'agobjfn_t' type signature
The `dict_relabel` function is used as a callback and the compiler correctly
identified it would be invoked incorrectly:
node.c: In function ‘agrelabel_node’:
node.c:245:37: warning: cast between incompatible function types from
‘void (*)(Agnode_t *, void *)’ {aka ‘void (*)(struct Agnode_s *, void *)’}
to ‘void (*)(Agraph_t *, Agobj_t *, void *)’ {aka ‘void (*)(struct Agraph_s
*, struct Agobj_s *, void *)’} [-Wcast-function-type]
245 | agapply(g, (Agobj_t*)n, (agobjfn_t)dict_relabel, &new_id, FALSE);
| ^
The calling convention of `dict_relabel` and `agobjfn_t` differed in a way that
meant the callback would result in stack corruption on most platforms.
Note that this has little effect on the #2300 test case involving `gxl2gv`
because `dict_relabel` is still incorrect, crashing with references to invalid
pointers when you invoke it.
I do not know why 978399c54faa413ec105fb5849b35f94ac17ff10 added this option to
force debugging information on and the commit message provides no enlightenment.
This squashes some warnings on Windows and 32-bit platforms:
generic_list.c(25,28): warning C4244: 'function': conversion from 'uint64_t'
to 'size_t', possible loss of data
generic_list.c(47,56): warning C4244: 'function': conversion from 'uint64_t'
to 'size_t', possible loss of data
generic_list.c(47,66): warning C4244: 'function': conversion from 'uint64_t'
to 'size_t', possible loss of data
This application has no ability to handle allocation failures. For example,
allocation failure during adding an attribute would result in setting
`attr_list` to null which would then go on to crash the program. Lets stop
pretending `prune` handles these cases and just exit on allocation failure.
tvnodes.c: In function ‘populate_data’:
tvnodes.c:195:48: warning: declaration of ‘grid’ shadows a global declaration
[-Wshadow]
195 | static void populate_data(Agraph_t * g, grid * grid)
| ~~~~~~~^~~~
tvnodes.c:32:3: note: shadowed declaration is here
32 | } grid;
| ^~~~
This seems a little overly prescriptive – one `grid` was a type and the other
was a value – but maybe the compiler is concerned that a future introduction of
a `sizeof(grid)` call in `populate_data` would be ambiguous.
By using string views, we can avoid the need to duplicate the graph attribute
value here. This allocation that was previously being lost no longer is made at
all.
smyrna: duplicate names when creating new grid columns
This is a step towards solving a memory leak. Note that this involved rewriting
some string comparisons because the originating pointer is no longer stored in
the column’s `name`. It is not clear to me how the previous code was valid
because, e.g., it was comparing by pointer to the `Visible` constant to detect
when a column was called “Visible”. That is, nothing actually assigns a column
name to the address of `Visible`.
smyrna: squash -Wconversion warnings related to grid column count
This squashes the following compiler warnings:
tvnodes.c: In function ‘update_tree’:
tvnodes.c:294:28: warning: conversion to ‘size_t’ {aka ‘long unsigned
int’} from ‘int’ may change the sign of the result [-Wsign-conversion]
294 | types = gv_calloc(g->count, sizeof(GType));
| ~^~~~~~~
tvnodes.c: In function ‘add_column’:
tvnodes.c:312:43: warning: conversion to ‘size_t’ {aka ‘long unsigned
int’} from ‘int’ may change the sign of the result [-Wsign-conversion]
312 | g->columns = gv_recalloc(g->columns, g->count, g->count + 1,
| ~^~~~~~~
tvnodes.c:312:61: warning: conversion to ‘size_t’ {aka ‘long unsigned
int’} from ‘int’ may change the sign of the result [-Wsign-conversion]
312 | g->columns = gv_recalloc(g->columns, g->count, g->count + 1,
| ~~~~~~~~~^~~
`count` is known non-negative all the time, but cannot be easily converted to a
`size_t` because we need to interact with the GTK APIs that take `int` values.
The last non-trivial call to `_sfpopen` was removed in f552c0dece1435773de62334dee3310d1728e22f. Since then, `proc` members have been
allocated and propagated around with data describing the absence of a subprocess
(`pid == -1`). We can reduce memory overhead and decrease code complexity by
removing these semantically dead code paths.
sfio: use a more portable way of determining page size
This code was using a legacy API, `getpagesize`, to opportunistically find the
system virtual memory page size. This change updates to the newer POSIX
`sysconf` API that is available almost everywhere, and adds a workaround for
Windows. This code now has an accurate understanding of the page size on
platforms that did not have `getpagesize` (of which, Windows was one), rather
than falling back on 8192.
673b9f1a7dbde9c9cc5d9a2a22ee835a08ab40ab tried to work around this in the past.
But there remained a foot gun. If you (possibly transitively) included sfio.h
but did not include sfhdr.h, you could end up facing this error.
Surprising as it may seem, nothing in the code base checks for `SF_ERROR`. So a
cleaner solution that removes the problem in perpetuity is to remove our
`SF_ERROR`. Note that `SF_FLAGS` does not need to be adjusted because `SF_ERROR`
was not a public flag.
CI: remove command line argument parsing from the deploy script
This script is called in exactly one place and always passed the `--verbose`
option. This change removes this only remaining option from the script and
unconditionally applies its effect, for a minor simplification.
projects / graphviz / log