[r13014]
by
mikeaubury
Fix the real bugs behind the remaining compiler warnings
These are the ones set aside earlier as "the interesting shortlist" - every
warning here was pointing at code that does not do what it was written to do.
A full clean build stays at exit 0 and drops from 754 to 721 warnings.
Memory:
- compilers/4glc/parsehelp.c: malloc(sizeof(expr_str)) followed by memcpy of
sizeof(l->list.list_val[a]) - the size of the *pointer*. It copied 8 bytes
and left the rest of the new expr_str uninitialised.
- lib/libaubit4gl/sql_common.c: A4GL_free_prepare() called
blank_any_cursors_using(sid) after free(sid). That function only compares the
pointer, so it now runs before the free - otherwise any cursor still holding
the sid keeps a dangling statement pointer.
- lib/libsql/odbc/sqlex.c: A4GL_debug("-%p", ptr) after free(ptr). Reading the
pointer value after the free is undefined even just to print it; logs first.
- lib/liblogical/processor/process_report.c: fclose() on a stream from popen().
Now pclose(), so the child is reaped and its status collected. The fwrite in
the copy loop was also inside an assert(), so a build with NDEBUG would have
dropped the copy entirely - it is now a separate statement.
- compilers/4glc/compile.c: two sprintf calls passing incl_path as both the
destination and the first argument. Overlapping source and destination is
undefined; they now format the suffix into a temporary and strcat it.
Logic that never ran, or always ran:
- compilers/4glc/lint.c: "whencode & 0x15 == WHEN_CALL" - 0x15 masks off bit 1
and WHEN_CALL is 2, so the WHENEVER ERROR CALL branch could never be taken and
lint never checked the named function existed. The action is the low nibble
(php.c uses & 0xf for the action, >> 4 for the condition), so it is now 0xf.
- compilers/4glc/lint.c: system_function_dtype() fell off the end without
returning. Callers test for -2, and the two other copies of this function
(calltree.c, prototypes.c) both end with return -2 - this one now does too.
- compilers/fcompile/dump_scr.c: get_attr_from_field() fell off the end when it
found nothing, returning whatever was in the register. Returns 0, which is
what its own "not applicable" path returns and what callers test for.
- lib/libaubit4gl/function_call_stack.c: "moduleName == '\0'" compared the
pointer against NULL, but its unset value is "" - so the no-module branch
never fired. Tests the string now.
- lib/libui/ui_xml/uilib/uilib.c and ui_json/uilib/uilib.c: "rval < 0" where
rval is the size_t returned by iconv(), which reports failure as (size_t)-1.
Conversion errors were silently ignored in both copies.
- compilers/4glc/variables_new.c: "if (idtype!=idtype)" - a self comparison, so
dead. What it meant is not recoverable; removed with a note.
- 16 -Waddress cases across sqlexpr.c, report.c, stack.c, mod.c, json.c, xml.c
and two generic_ui.c copies: tests on the address of a struct's char array,
which is never NULL. Each had a real test beside it (strlen, [0], or a
function call), so the redundant half is gone and the meaning is unchanged.
Format strings:
- compilers/4glc/prototypes.c: three fprintf calls passing arguments to format
strings with no conversions at all.
- tools/asql/parse.l: "%d" given strlen() and sizeof() results, ie size_t.
Left alone deliberately: compile_c.c's "arr_subscripts_len >= 0" was always true
on an unsigned member, but changing it to "> 0" would alter what happens for an
empty subscript list, so only the dead half was dropped.
|
2026-08-27 12:18:56
|
Tree
|
[r13013]
by
mikeaubury
Drop the Glade IDE from the default build
glade/ide and glade/ide/sqleditor are no longer built by "make". They have not
been used in years, and keeping them in the default target means everyone pays
for them: the sqleditor sources are a 2017 fork of the tools/asql ones that has
since drifted, and their generated code calls aclfgl_* functions in sibling
modules with no prototypes in scope - 32 diagnostics that GCC 14 and later
treat as errors rather than warnings. Fixing those properly means working out
why tools/asql does not have the same problem, which is not worth doing for a
component nobody runs.
Only the ALL list changes. The "glade" target itself is untouched, so "make
glade" still builds it for anyone who wants it, and "make clean" still cleans
it. Nothing else references it.
With this, a full clean build of the default target is free of every diagnostic
GCC 14+ turns into an error - in the compiler output and in the .warn files
4glpc writes, which is where the last few rounds of these were hiding. 49 .warn
files, none with a fatal diagnostic; build exits 0 at 754 warnings; asql,
adbload2 and the compiled 4gl test programs all still run.
|
2026-08-27 11:53:46
|
Tree
|
[r13012]
by
mikeaubury
Clear the GCC 14+ errors from the rest of corecompile: adbload2 and runforms
Continues r13011. The .warn files that 4glpc writes - which never reach the
build log - had the same class of problem in the two directories corecompile
reaches after tools/asql. Both are now free of diagnostics that GCC 14+ treats
as errors; a full clean build stays at exit 0.
tools/adbload2 (22 down to 0):
- process.ec and jtypes.c call runtime entry points with no prototype in scope.
Declared in a4gl_dbload_int.h rather than by including a4gl_libaubit4gl.h,
because that header defines A4GL_assertion as a two argument macro and
jtypes.c calls it with parenthesised arguments the preprocessor miscounts.
The block is guarded on _AUBIT_LIB_INCL_EXT_ so the translation units that do
include the header keep using its versions and do not get redundant
declarations.
- Forward declarations for the functions defined further down process.ec, and
for those in jtypes.c, which is #included near the bottom of it - so a
declaration in jtypes.c itself comes too late for callers above.
- Declaring A4GL_assertion turned up three calls that never matched it: one
passing no message at all, and two passing a printf-style format plus an
argument to a function that takes (int, char *) and does no formatting. They
now build the message with snprintf first.
- Three A4GL_assertion calls passed dbload->f->delim, a char *, as the int
condition; they mean "is it set", so they now say != NULL.
tools/asql/runforms (3 down to 0):
- A4GL_isnull, A4GL_read_columns and aclfgl_a4gl_get_info were declared only in
a4gl_libaubit4gl.h, which generated 4gl code does not include - it takes the
a4gl_incl_4glhdr.h path. Moved to a4gl_incl_4gldef.h, which is where CLAUDE.md
says shared declarations belong. Signatures checked against stack.c,
sql_common.c and helper.c rather than copied from the header.
- dbi.4gl passes &lv_dtype and &lv_size to A4GL_read_columns, which writes ints,
but 4gl integers are longs: the top four bytes were left uninitialised on
LP64. Same bug and same fix as the sqgetdbs calls in r13011.
Also in lib/libaubit4gl/ui.c: A4GL_open_gui_form declared its last parameter as
"void (*handler_c (int a, int b))", ie a function returning void *, while every
caller passes int (*)(void *, void *) and the function it forwards to,
A4GL_open_gui_form_internal, takes a plain void * - as does handler_e beside it.
Made it void *. That declaration moved to a4gl_incl_4gldef.h too, since the
callers are generated 4gl code.
Not done: glade/ide and glade/ide/sqleditor still have 32, mostly generated 4gl
code calling aclfgl_* functions from sibling modules without prototypes. glade
is a separate target from corecompile, built only when enable_minimal is not
set, so it does not block the core build.
One thing tried and reverted: declaring compat.c's bare trim() in
a4gl_libaubit4gl.h. The name is too common - lib/liblogical/txt/process has its
own static trim() - and it broke the build. Callers that want the library one
declare it themselves; there is now a comment in the header saying so.
|
2026-08-27 11:38:50
|
Tree
|
[r13011]
by
mikeaubury
Fix the GCC 14+ errors in tools/asql, and a real sqgetdbs/long bug with them
Reported building r13010: "Error compiling infx.ec" in tools/asql, with no
detail. The detail is in tools/asql/infx.warn, which 4glpc writes instead of
sending to stdout - that is why none of this showed up in the build log.
Three things in there are warnings under GCC 13 and errors from GCC 14 on:
- jtypes.c calls isxdigit, isspace, isalnum, isdigit, isalpha, ispunct and
tolower with only <unistd.h> included. Added <ctype.h>. There are two copies
of this file, tools/asql and tools/adbload2, and both had it.
- infx.4gl defines execute_select_free() and asql_explain() with no return
type. Both are used as int by their callers (execute.c) and declared int in
generic.c, so they are now written that way. asql_explain also had no return
statement at all while execute.c does "raffected=asql_explain(...)", so it
now returns 0 rather than whatever was in the register.
- sqgetdbs() is exported by the ESQL/C library but declared in none of the
shipped headers, so it was an implicit declaration. Added a prototype, the
same one added to lib/libsql/esqlc/esql.ec in r13002.
Declaring sqgetdbs then exposed a genuine bug at three of its four call sites.
It writes an int through its first argument, but they pass &ndbs where ndbs is
a 4gl integer, ie a long: on LP64 the top four bytes are left uninitialised and
the database count comes back wrong. The fourth call site already worked around
this with an int temporary; the other three now do the same.
Verified by rebuilding tools/asql: infx.warn goes from 10 GCC-14-fatal
diagnostics to 0, and a full clean build still passes.
Note for whoever picks this up next: a sweep of the .warn files this build
writes shows the same class of problem still present in tools/adbload2 (22),
glade/ide (about 26 across several files) and tools/asql/runforms (3) - all of
them functions called without a visible prototype. They are next in line to
stop a GCC 15 build.
|
2026-08-27 11:06:19
|
Tree
|
[r13010]
by
mikeaubury
Make bin/aubit POSIX sh, so it no longer depends on bash
The wrapper could not be parsed by a POSIX shell at all - dash stopped at line
39 on "function dummy ()". It coped by re-execing itself:
if [ -z "$BASH_VERSION" ]; then
exec bash "$0" "$@"
fi
which only works where bash exists. That is not everywhere we claim to build:
Alpine/musl images frequently have no bash, and FreeBSD keeps it in
/usr/local/bin. It also cannot help the case that matters most - incl/Makefile-
common sets SH=sh, so the makefiles run "${SH} .../bin/aubit fcompile ..." and
a shebang would be ignored there anyway. On any system where /bin/sh is dash,
those rules were relying on that exec finding a bash.
The script turned out to be very close to POSIX already - no [[ ]], no arrays,
no ${x//y}, no +=, no <<<, no brace expansion. What it used:
118 function NAME () -> NAME ()
44 let X=X+1 -> X=$((X+1))
6 echo -e -> printf '%b\n' (dash's echo expands escapes
itself and would have printed a literal "-e")
4 source F -> . F
4 declare X=Y -> local X=Y (matching the comment above them)
With those gone the re-exec is unnecessary and is removed, and the file gets a
real "#!/bin/sh" on line 1 - the one it had was on line 32, where it is just a
comment. The dummy() tripwire stays as a guard for a shell with no function
support at all, but its message no longer tells people to install bash, and it
no longer tries to run itself under a bash it may not have.
Verified rather than assumed: bin/aubit now parses under dash, bash and sh;
"aubit-config", "4glpc", "fcompile" and "amkmessage" all produce correct output
when driven by dash as well as bash; and tools/test builds end to end with
"make SH=/bin/dash SHELL=/bin/dash", which exercises the wrapper for form
compilation and message compilation. Before this change dash could not get past
parsing the file.
A full clean build is unaffected: 724 warnings, exit 0.
|
2026-08-26 07:59:53
|
Tree
|
[r13009]
by
mikeaubury
Stop using "which" to find commands - it is not guaranteed to exist
Follow-on audit after r13008. The flexml failure was one instance of a general
habit: asking the shell whether a command exists in a way that is not portable.
"which" is an external program, not a shell builtin, and Debian dropped it from
debianutils in 13 - so on the newest Debian and Ubuntu it may simply not be
installed. Every `which foo` then produces nothing, and the surrounding code
concludes the tool is absent (or, where the result is used as a path, gets an
empty one). That is the same shape of bug as r13008, just failing the other way
round: there, a missing tool looked present; here, a present tool looks missing.
Converted to "command -v", which is POSIX, is built into every shell, and
prints a path on stdout only when the command exists:
- bin/aubit: 26 call sites - the compiler wrapper's probes for gcc, make,
ldconfig, locate, xdg-open, xmessage, desktop-file-install, 4glc, 4glpc and
the rest. This is the script CLAUDE.md tells everyone to invoke, so it is the
one that matters most.
- Makefile: the check.exe.in.path target, which exists to refuse to install
when Aubit executables are still on PATH. With "which" gone that check
silently passes, which is exactly backwards for a safety check.
- bin/aubitbuild.sh.in: was deriving a script's path by picking field 3 or 4
out of "type" output, with a fallback because the field moves depending on
whether the command is hashed - and the wording differs between shells
anyway. command -v gives the path directly. Patched in the .in template, as
configure generates bin/aubitbuild.sh from it.
Two things found in passing and deliberately left alone, both worth a look:
- bin/aubit line 806 reads x="`command -v $prg >/dev/null 2>&1`" - stdout goes
to /dev/null, so x is always empty and the "Found in PATH" branch below it
has never run. Removing the redirect would switch on a branch that does
rm -rf on what it finds, under A4GL_FORCE_CLEAN, so that is a decision to
make deliberately rather than as part of a mechanical sweep.
- bin/aubit has no shebang: the "#!/bin/sh" in it is on line 32, not line 1.
It also does not parse under dash (bash-isms), so it relies on being started
by a bash-compatible shell. It works today because whatever runs it is bash;
it would break where /bin/sh is dash and something execs it directly.
The one remaining "type" probe is PKG_CONFIG_ROOT_CYGPATH in
incl/Makefile-install.mki, which parses type's output for a Cygwin path.
Cygwin is no longer a supported platform, so it is left as it is.
|
2026-08-26 07:47:15
|
Tree
|
[r13008]
by
mikeaubury
Detect optional build tools with "command -v", not "type"
Reported building r13007 on Ubuntu 26.04: the build stops in
lib/libui/ui_xml/uilib because it tries to run flexml, which is not installed.
It should never have tried. uilib/Makefile already falls back to the
checked-in xml/pregenerated/ copies of triggers.c, triggers_act.c and
triggers.h when flexml is missing:
HASFLEXML:=$(shell type flexml 2> /dev/null)
ifeq "$(HASFLEXML)" ""
TRIGGERS_O=xml/pregenerated/triggers.o xml/pregenerated/triggers_act.o
but the probe is not safe. "type" is a shell builtin whose behaviour for a
missing command differs: bash writes "flexml: not found" to stderr, so the
redirect hides it and HASFLEXML ends up empty, while dash - which is /bin/sh
on Debian and Ubuntu - writes it to stdout, where the redirect does not touch
it. HASFLEXML is then the non-empty string "flexml: not found", the makefile
concludes flexml is available, and make runs it and fails:
$ dash -c 'type flexml 2> /dev/null'
flexml: not found
$ dash -c 'command -v flexml 2>/dev/null'
$
"command -v" is the POSIX way to ask this and prints a path on stdout only
when the command exists, on either shell.
Fixed in all ten probes of this shape, not just the one that bit:
lib/libui/ui_xml/uilib/Makefile and uilib/xml/Makefile (flexml),
lib/libpacker/xml_best, xml_best2 and xml_best_plsql (flexml and a pregenerated
flex), and lib/extra_libs/mantisconnect (soapcpp2). All had the same latent
failure - each would have tried to run a tool that is not there.
Not reproduced end to end here: this box has bash as /bin/sh and GNU Make 4.3,
where the old probe happens to come back empty and the pregenerated files are
used, which is why the tree built fine for me at r13006. What is demonstrated
is the probe itself being unsafe under dash, and that both shells now select
xml/pregenerated/. The exact combination on the reporter's machine (dash plus a
newer make) is inferred.
The warnings quoted alongside the failure - comms.c sign-compare and unused
'p', uilib.c type-limits, attr.c redundant xml_yylex - are untouched; they are
warnings, not the reason the build stopped.
|
2026-08-25 16:10:04
|
Tree
|
|
|
2026-08-25 14:17:26
|
Tree
|
[r13006]
by
mikeaubury
Build haru.c against libharu 2.4 as well as 2.3
Reported against r13001 on Ubuntu 26.04, once the libcurl failure was out of the
way: lib/libpdf/haru.c does not compile against the libharu in /usr/local.
libharu 2.4 changed two things this file relies on:
- HPDF_Page_SetDash() takes its pattern as const HPDF_REAL * and its phase as
HPDF_REAL. Up to 2.3 those were const HPDF_UINT16 * and HPDF_UINT, and the
dash_mode arrays here are declared HPDF_UINT16 - a hard error under GCC 14+,
which is what the reporter hit at haru.c:3940 and :4012.
- HPDF_PROJECTING_SCUARE_END was corrected to HPDF_PROJECTING_SQUARE_END, so
haru.c:4069 no longer names anything that exists.
Rather than move to the new spelling and break older installations, pick both
from the version macros in hpdf_version.h (which hpdf.h includes): the dash
arrays use A4GL_HPDF_DASH, HPDF_REAL from 2.4 onwards and HPDF_UINT16 before
that, and the code now uses the corrected SQUARE spelling with a #define
mapping it back to SCUARE on older headers. A build with neither macro defined
gets the old API, which is what it would have had anyway.
Note that on 2.4 the dash lengths stop being truncated to whole points, since
the pattern is now float either side of the call.
Verified both ways: compiles clean against the 2.3.0 headers on this box (the
old branch, including the SCUARE remap), and the preprocessor picks HPDF_REAL
plus the corrected spelling when the version macros say 2.4. I have no 2.4
install here to compile the new branch end to end, so that half is verified at
the preprocessor rather than by building it.
The comment at the old line 4059 ("SCUARE is not a spelling mistake!") now
points at the shim instead - it was true of 2.3 and wrong from 2.4 on.
|
2026-08-25 14:11:53
|
Tree
|
[r13005]
by
mikeaubury
Finish the sprintf conversion in the grammar fragments and lint.c
Follow-on to r13004. -Wformat-overflow goes from 14 to 7, and a full build from
737 to 730 warnings.
GCC only flags the calls whose overflow it can demonstrate; the rest of the raw
sprintf() calls in the same files are exactly as unchecked, they just have
arguments it cannot bound. So rather than convert the seven it named, this
converts every remaining sprintf() in the files that owned them - 44 calls in
compilers/sql/80.reqd, compilers/sqlcmd/80.reqd,
compilers/4glc/rules/sqlpack/infx/sql1.rule and compilers/4glc/lint.c.
Every destination in those files is either a local char buff[N] or the parser's
$<str>$, which is the char str[1024] member of the %union - both arrays, so
sizeof() in the SPRINTFn macro measures the real space and A4GL_sprintf raises
its assertion, naming file and line, when the result will not fit. Which is the
point: we do not need to know in advance whether a 2k buffer can overflow, we
need it to stop loudly on the day it does.
(lint.c has one static char *buff, in local_xml_escape(), which builds its
string by hand and is not a sprintf destination - so no call was converted to a
macro that would silently take the unchecked pointer path.)
The remaining 7 are the two files that cannot use the macro at all, as noted in
r13004: compilers/xgen/x.yacc (5), where bin/xgen is a bootstrap tool linking no
library, and lib/libui/ui_xml/proxy.c (2), which includes only system headers.
|
2026-08-25 13:56:51
|
Tree
|