When global <sys/mman.h> was removed from tile.h it also removed
<unistd.h> nearby. This exposes the lack of <unistd.h> being
included where needed using APIs like close()/read()/unlink()/isatty()
the WASM build seems to show this as the header file set is structured
differently.
Original version would iterate exhaustively, even when it was not
necessary.
This version seeks to do the minimum amount of iteration work based
on the information available.
There are numerous concerns with the original code from a read through.
The #define TX_MAX_OPEN_FILES 20 is badly named, the txCommand.c uses
fd_set to hold active FD for each device, and makes no attempt to bounds
check the 'fd' or 'fd_set' of any TxAdd1InputDevice() is in range.
The real name of this variable should be TX_MAX_INPUT_DEVICES as it
related to the fixed compile time slots available for devices, each input
device must have at least one active FD and it can be in the range
that fd_set allows, which is 0..1023 on linux.
So based on this being the original intention, due to the way the code is
constructed and API calls made available, the file has been reworked
to allow all these considerations at the same time.
Now the design should be FD clean for FDs in the range 0..1023 instead of
being artificially clamped to the first 20 FDs being valid input devices.
Renaming of variable 'i' to 'fd' when it relates to an fd number, so all
uses of FD_XXXX() should use fd, this helps clarify the different domain
the number relates to.
When 'i' is used it relates to the index into the txInputDevRec array.
A large part of the concerns relate to trying to mix the two numbering
domains interchangeably, just using a different name really clears up
understanding to the reader.
Other items that look like errors TxDelete1InputDevice() will
shuffle-remove entries with no active FD, but it is iterating an array
it is also modifying, so it looks like it would have incorrectly skipped
the next item that should be inspected just after a shuffle-removal
occurred.
The various iterators that works from 0..TX_MAX_OPEN_FILES incorrectly
limited the visibility of the routines to the first 20 FDs instead of
the full FD_SETSIZE the TxAddInputDevice API call allows to be
registered.
Passing in TxAdd1InputDevice with an fd above 19 would have resulted in
everything looking successful until select() was used, then the kernel
would likely not sleep/wait because the input fd_set would look blank due
to being clipped by nfds=TX_MAX_OPEN_FILES (instead of that plus 1).
The use of TX_MAX_OPEN_FILES in select instead of TX_MAX_OPEN_FILES+1 for
the 'nfds' field would have meant a device with fd=19 would not work as
the design intended.
The define definition has been removed from the module public header,
I assume it was there for use by another module, but the incorrect
select() usage has been corrected over there in a previous commit.
So now the define can be maintained near the array it relates to.
While the code might looks less 'efficient' as it is now sweeping all
1024 FDs when input devices during add/remove (hopefully there maybe some
compiler auto-vectorization/tzcnt use there). The main event loop is
slightly more 'efficient' (especially for the single device with input
fd=0 case) as it will only check the 1 FD each input event loop iteration,
not check all 20 FDs for data readyness everytime.
This encapsulates the expectation the 'fd' is in the permitted range for
the standard sizes 'fd_set'.
This is so there is some form of detection with issues in this area, if
the RLIMIT_NOFILE limit is increased.
interactive wiring into coordinate-based commands. Added new
command extensions for "wire leg", "wire vertical", "wire type",
and "wire horizontal". Modified the command logging such that
"wire show" (which does not modify layout) does not get logged,
which avoids unnecessary logging of mouse movement.
generally unused, as it is incompatible with the Tcl/Tk build of
magic. However, I had not intended to remove it, only move the
name from "readline-4.3" to "readline". But "readline/readline"
was in .gitignore, which caused the contents to be removed from
the repository. This commit restores those files, and also
prevents the readline directory Makefile from making a symbolic
link called "readline" to itself, which was causing compile-time
issues. The readline code is only kept for backwards compatibility
with ancient versions of magic not using the Tcl/Tk interpreter.
This seems like it has 2 use cases.
Internal console management around reprinting command prompt, but many
modes of operation delegate the prompt processing to tkcon or readline.
Process termination to restore the termios.
This commit related to the dynamic creation of data that is used
to parse commands and options via Lookup.
windows/windows.h: Lookup() constify call-site
tcltk/tclmagic.c: Lookup() constify call-site
graphics/W3Dmain.c: Lookup() constify call-site
windows/windSend.c: Lookup() constify call-site
windows/windMain.c: Lookup() constify call-site
windows/windInt.h: Lookup() constify call-site
textio/txMain.c: Lookup() constify call-site
Maybe ASSERT are not always active, so defensive coding solution.
txCommands.c:883:6: warning: variable 'but' is used uninitialized whenever switch default is taken
txCommands.c:888:6: warning: variable 'act' is used uninitialized whenever switch default is taken
clang18 -Wall warning cleanup [-Wsometimes-uninitialized]
This function is related to libreadline rl_pre_input_hook callback
which is invoked as (not making use of any function return value):
readline.c: (*rl_pre_input_hook) ();
The general prototype for this function is:
rltypedefs.h:typedef int rl_hook_func_t PARAMS((void));
So the resolution is to provide a known value as the return value, which
resolves the concern.
SonarCloud
textio/txInput.c:550 non-void function does not return a value
https://sonarcloud.io/project/issues?open=AZJB17NwNGfDNup0Rj5G&id=dlmiles_magic
Fix code scanning alert no. 116: Wrong type of arguments to formatting function (#19)
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Fix code scanning alert no. 115: Wrong type of arguments to formatting function (#20)
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Fix code scanning alert no. 114: Wrong type of arguments to formatting function (#21)
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
"pick x y" which acts like "cursor", but operates on a database
coordinate instead of a pointer coordinate. Made a few other
corrections to the command logging code so that it produces
valid output when the log file is sourced.
been broken ever since moving to the Tcl/Tk wrapped version. Added
some new features that allow background commands from the window
handling (like pointer tracking) to be omitted from the log file
via a suspend/resume function. Added a header file and a few
commands at the top of the log file that align the log file contents
with the screen and box state at the start of logging. This makes
a log file which can be "played back" by sourcing it from the magic
console prompt. Per request from Harald Pretl.
Darryl L. Miles
Tim Edwards
Johan Euphrosine