feat: migrate all write flag and cnt0 accesses to atomic macros#247
Closed
oguyon wants to merge 370 commits into
Closed
feat: migrate all write flag and cnt0 accesses to atomic macros#247oguyon wants to merge 370 commits into
oguyon wants to merge 370 commits into
Conversation
…eiver-side create_image_ID -> imgid_mkimage\n Uses remote imgmd metadata for dimensions, datatype, shared, NBkw\n- stream_UDP.c: same pattern for UDP stream receive"
…gebra files Convert remaining create_image_ID calls in: - cublas_linalgebratest.c (3 calls) - magma_compute_SVDpseudoInverse_SVD.c (2 calls) - magma_compute_SVDpseudoInverse.c (1 call) - magma_MatMatMult_testPseudoInverse.c (1 call) - cublas_PCA.c (1 call) - GPU_SVD_computeControlMatrix.c (1 call) - GPU_loop_MultMat_setup.c (1 call)
- Create milk_compiler.h with GCC performance macros (LIKELY/UNLIKELY, MILK_HOT/COLD/PURE/CONST/FLATTEN, MILK_IVDEP, MILK_ALIGNED, MILK_RESTRICT) - Apply MILK_HOT to all compute_function() and fpsexec() (156 functions) - Apply MILK_COLD to all customCONFsetup/check() (10 functions) - Apply MILK_PURE to image_ID_noaccessupdate() - Apply MILK_IVDEP to 5 pixel loops in image_pixunmap/pixremap - Fix ~142 float literals in .array.F[] contexts (0.5 -> 0.5f) - Add -funroll-loops to Release flags - Add USE_PGO CMake option (GENERATE/USE) with docs/pgo.md - Add performance-practices.md agent rule - Update code-style-guide.md with restrict guidance - Remove local likely/unlikely macros from logshmim.c
- Add milk_pgo_target() helper to MilkStandalone.cmake - Each standalone gets its own profile subdirectory under PGO_DIR (default: _build/pgo/<exe-name>/) - Shared libraries use pgo/shared/ (aggregated) - Applied to add_milk_standalone, add_cacao_standalone - Updated docs/pgo.md with per-executable workflow
Engine layer (18 removals):
- ImageStreamIO.c: math.h, pthread.h, signal.h, sys/file.h,
arpa/inet.h, netinet/in.h, netinet/tcp.h, duplicate unistd.h
- milk-fps-set.c: math.h, ctype.h
- procCTRL_TUI.c: sys/file.h, math.h, ctype.h (x2), pthread.h
- processinfo_shm_{create,link,list_create}.c: sys/file.h
CLI layer (3 removals):
- CLIcore.c: pthread.h
- CLIcore_help.c: assert.h
- streamCTRL_TUI.c: sys/file.h (added explicit fcntl.h)
Core modules (15 removals):
- COREMOD_arith.c: ctype.h
- 5 image_arith*.c files: math.h
- image_dxdy.c: assert.h
- image_unfold.c: math.h
- imfunctions.c: assert.h
- image_complex.c, image_mk_{complex_from_reim,reim_from_complex}.c: math.h
- stream_UDP.c: netinet/tcp.h
- stream_delay.c: math.h
- Add simd to all 109 #pragma omp for directives - Add restrict to 42 local pixel pointer declarations - Cache getpid() as thread-local in ImageStreamIO_sempost - Replace printf with ImageStreamIO_printERROR in sempost - Add MAP_HUGETLB opt-in via MILK_SHM_HUGETLB=1 env var with automatic fallback (streams >= 2MB only) - Add VEC_REPORT CMake option (-fopt-info-vec-missed) - Document MILK_SHM_HUGETLB in docs/performance.md - Update performance-practices.md agent rule
- Submodule ImageStreamIO updated with optimizations - Added -D to milksemloopspeed test script in CMakeLists.txt to pass ctest
- Applied MILK_ASSUME_ALIGNED to all generated loop point-ops within COREMOD_arith module - Updated performance-practices.md adding guidance on transcendentals and LUTs - Additional SIMD optimization updates in core memory routines
Add performance-practices rule, run-milk-commands rule, and new workflows (add-function, add-stream-processor, add-cli-command). Add relative links to all rule and workflow files.
- Add UINT16 typed fast-path with MILK_RESTRICT and MILK_ASSUME_ALIGNED to stream_copy.c alongside the existing FLOAT path. - Update performance-practices.md rule with new sections: pow() specialization, Memory Allocation (no malloc in hot loop), and Typed Fast-Paths guidance.
- MVM_CPU.c: Replace naive O(MN) loop with cblas_sgemv() when MKL or OpenBLAS available. Plain-C fallback uses restrict + OMP + SIMD. Remove printf from hot path. - imfunctions.c: Change all if→else if chains in 7 datatype-dispatch functions to skip ~9 redundant comparisons per call.
Add two new sections to performance-practices.md: - BLAS for Matrix Operations: use cblas_sgemv/sgemm - Datatype Dispatch: use else-if not bare if Also add matching entries to What NOT to Do.
- AGENTS.md: Add 2 missing rules to section 6 table (files-directories.md, performance-practices.md) - code_assist.md: Update performance rule description to reflect expanded scope (BLAS, type dispatch, memory allocation) - GEMINI.md: Add V2 poc template reference, link to performance-practices.md, pointer to AGENTS.md - performance.md: Add cross-reference to code-level optimization rules and code_assist index
New rule enforcing that all code changes go through pull requests from feature branches into framework-dev. Updated AGENTS.md and code_assist.md to list it.
Merging Sweep 4 memory and pointer optimizations.
* docs: add naming conventions rule and expand code style guide Add comprehensive naming-conventions.md agent rule covering: - File, directory, function, and variable naming patterns - Loop index type-matching for GCC auto-vectorization (uint32_t index vs uint64_t bound causes 8x slowdown) - C/POSIX reserved namespace awareness (_DATATYPE_* legacy) - Approved abbreviation list (guidance, used by agents) - Dimension variable standards matching IMAGE_METADATA fields - FPS parameter keyword conventions Expand code-style-guide.md with 4 new rules: - Factorize code: extract helpers instead of copy-paste - Doc placement: purpose in .h, implementation details in .c - Closing scope comments for blocks longer than ~10 lines - Early exit preferred over deeply nested braces Change line width limit from 80 to 100 characters for both C source and agent markdown files. Updated consistently across 9 files (rules, skills, workflows, AGENTS.md, code_assist.md). Add loop index type-matching rule to performance-practices.md. * Update AGENTS.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update .agents/rules/code-style-guide.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update AGENTS.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update .agents/rules/naming-conventions.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update .agents/rules/naming-conventions.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update .agents/rules/naming-conventions.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * docs: fix Public API naming pattern to fully-lowercase snake_case Agent-Logs-Url: https://github.com/milk-org/milk/sessions/b580d751-71be-49d1-be52-ed0ecd48d1a1 Co-authored-by: oguyon <5598733+oguyon@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: oguyon <5598733+oguyon@users.noreply.github.com>
* refactor: replace duplicated tab-bar rendering with X-macro table\n\nThe six copy-pasted if/else blocks that highlight the\nactive tab in streamCTRL_TUI.c (72 lines) are replaced\nby a TAB_LIST X-macro and a RENDER_ONE_TAB expansion\n(13 lines). Adding a new tab is now a single row in the\ndeclarative table.\n\nNo functional change. * Update src/cli/streamCTRL/streamCTRL_TUI.c Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * refactor: drive CTRL+L/R bounds and comment from TAB_LIST X-macro - Update TAB_LIST comment to list all places that need updating when adding a new tab (header constant, key handler, help entry) - Add TAB_COUNT macro derived from TAB_LIST for CTRL+L/R wrap bounds - Replace hard-coded < 1 / > 6 navigation bounds with < DISPLAY_MODE_HELP / > TAB_COUNT driven by the X-macro - RENDER_ONE_TAB do-while(0) wrapping already present from previous commit Agent-Logs-Url: https://github.com/milk-org/milk/sessions/f669e408-48c1-4a11-a0ab-ea8a94a5e6c1 Co-authored-by: oguyon <5598733+oguyon@users.noreply.github.com> * fix: use compound statement instead of do-while(0) in X-macro callback\n\ndo{...}while(0) cannot be used as an X-macro callback\nbecause TAB_LIST(RENDER_ONE_TAB) concatenates multiple\ninvocations without semicolons between them, producing\ninvalid syntax. Use a plain compound statement { }\ninstead, which is valid when concatenated.\n\nFixes CI build failure from previous commit. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: oguyon <5598733+oguyon@users.noreply.github.com>
… bindings (#240) * feat(fps): add --force to milk-fps-rm and correct pointer binding docs * fix(fps): add pid>0/getpgid guards and kill() error handling in milk-fps-rm Agent-Logs-Url: https://github.com/milk-org/milk/sessions/6413f660-7f84-4552-b6ab-80410bc6d1cb Co-authored-by: oguyon <5598733+oguyon@users.noreply.github.com> * refactor(fps): simplify kill_proc - remove redundant existence check, save errno Agent-Logs-Url: https://github.com/milk-org/milk/sessions/6413f660-7f84-4552-b6ab-80410bc6d1cb Co-authored-by: oguyon <5598733+oguyon@users.noreply.github.com> * fix(fps): check ESRCH specifically in kill probes, remove redundant existence checks Agent-Logs-Url: https://github.com/milk-org/milk/sessions/6413f660-7f84-4552-b6ab-80410bc6d1cb Co-authored-by: oguyon <5598733+oguyon@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: oguyon <5598733+oguyon@users.noreply.github.com>
* fix: suppress spurious SHM warnings in TUI Added access() pre-checks using ImageStreamIO_filename() before calling ImageStreamIO_openIm() across libfps and fpsCTRL display routines. This prevents "Cannot open shm" ANSI-colored warning messages from polluting stderr and corrupting the ncurses display when checking uninitialized stream parameters. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix: syntax and struct errors in SHM TUI suppression Fixed compiler errors introduced by Copilot autofix: - fps_print_info.c: Replaced hallucinated FPS_STREAMNAME type with proper use of fps_streamname_parse returning FPS_STREAMNAME_PARSED by value. - fps_loadmemstream_lite.c: Restored the access() F_OK guard in the pure library fallback path while keeping the improved ImageStreamIO_filename path generation logic. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…241) * feat(fps): add --force to milk-fps-rm and correct pointer binding docs * diag: improve FPS stream error messages with caller context and empty-name detection fps_loadstream.c: - Annotate per-stream probe lines with [empty], [RUN-REQUIRED], [CONF-REQUIRED] tags for immediate visibility - Empty streams now print \"[skipped: parameter not configured]\" instead of \"NOT FOUND\" - ABORT messages for required streams now include the FPS parameter key and a milk-fps-set fix command imageID.h (resolveIMGID): - Substitute \"<empty name — FPS stream parameter not set>\" when img->name is NULL or empty, so ERROR/WARNING lines are never blank imageID.h (stream_connect_create_2D / stream_connect_create_2Df32): - Converted from static inline functions to macros so __FILE__, __LINE__, __FUNCTION__ are captured at the call site - Implementation moved to _stream_connect_create_2D_impl() - On empty/NULL stream name, abort message now prints the exact source location of the bad call, allowing developers to trace back to the FPS parameter key via the module variable name" * Update src/coremods/COREMOD_memory/imageID.h Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(fps-rm): add pid>0 guard, EPERM handling, and proper 2s polling wait loop for --force Agent-Logs-Url: https://github.com/milk-org/milk/sessions/72ca80ec-d3ec-4bde-9016-45bff865b453 Co-authored-by: oguyon <5598733+oguyon@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: oguyon <5598733+oguyon@users.noreply.github.com>
…p exit (#242) * fix: add caller context to resolveIMGID abort message Provides actionable diagnostics when a stream name is empty or unresolved by renaming `resolveIMGID` to `_resolveIMGID_impl` and generating a macro wrapper that passes `__FILE__`, `__LINE__`, and `__FUNCTION__`. This replaces generic aborts with clear error messages pointing exactly to the module failing to find a required FPS stream parameter. * feat: add fpskeyword field to IMGID for FPS stream diagnostics\n\nAdd fpskeyword[128] to the IMGID struct so that when\nresolveIMGID aborts on an empty stream name, the error\nmessage includes the exact FPS parameter key (e.g.\n\"mfilt-00.inmval\") and a suggested milk-fps-set command.\n\n- Add imgid_setfpskeyword() to tag an IMGID after creation\n- Add imgid_make_from_fpskey() convenience constructor\n- Update resolveIMGID abort path to print the FPS key" * fix: standalone compute loop exits after 1 step\n\nThe standalone version of INSERT_STD_PROCINFO_COMPUTEFUNC_INIT\nin fps_procinfo_macros.h was missing the line that ORs\nCLICMDFLAG_PROCINFO from dcfpsptr->cmdset.flags into\nCLIcmddata.cmdsettings->flags.\n\nWithout this, the LOOPSTART macro never sees the PROCINFO\nflag, so it sets processloopOK=0 on the first iteration,\ncausing standalone V2 executables (dmcomb, mfilt, etc.) to\nrun for exactly 1 step and exit.\n\nThe CLI version (CLIcore_utils.h) already had this line." * Update src/coremods/COREMOD_memory/imageID.h Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * refactor: replace hard-coded 128 with IMGID_FPSKEYWORD_STRMAXLEN constant Define IMGID_FPSKEYWORD_STRMAXLEN = STRINGMAXLEN_FPS_NAME + FUNCTION_PARAMETER_KEYWORD_STRMAXLEN (= 164) by including fps_types.h, replacing the hard-coded char fpskeyword[128] in the IMGID struct. This addresses reviewer feedback: the buffer size is now computed from existing FPS string-length limits, avoids silent truncation for long FPS names/keywords, and stays in sync if those limits ever change. Agent-Logs-Url: https://github.com/milk-org/milk/sessions/9812fdda-5594-4241-9609-68e02de7aea2 Co-authored-by: oguyon <5598733+oguyon@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: oguyon <5598733+oguyon@users.noreply.github.com>
* fix: resolve milk-fpsCTRL segmentation fault when FPS are removed * perf: replace per-node memset loop with single bulk memset in fps_scan Agent-Logs-Url: https://github.com/milk-org/milk/sessions/a23a8e8c-6f7e-4a87-aa75-16f3fd1dfed9 Co-authored-by: oguyon <5598733+oguyon@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: oguyon <5598733+oguyon@users.noreply.github.com>
…ut to milk-cli (#244) * feat: add comment handling, numerical assert, and full-precision output to milk-cli - Silently skip lines starting with '#' as comments - Highlight comment lines in dim green while typing - Add 'assert expr=expected ~tolerance' numerical check with bold green PASS / bold red FAIL output - Add 'assert expr<val', 'assert expr>=val' comparisons - Add 'assert lo<expr<hi' range checks (double inequality) - Upgrade assert [condition] to also show colored output - Fix math expressions (e.g. a=23.4) being flagged red - Fix array vs math expression disambiguation for a=(1+2)/3 - Switch double formatting from %g to %.15g throughout - Register 'assert' as internal keyword to avoid shell bypass - Add 18 new CLI robustness tests for assert and comments * Update tests/cli/cli_robustness_tests.milk Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/cli/libmilkscript/CLIcore_script_intercept_process.c Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix: move array-assign math disambiguation check before cli_arrays mutations In cli_try_array_assign(), the check for ')' followed by non-whitespace (e.g. a=(1+2)/3) now happens via a pre-scan BEFORE any cli_arrays slot is created or modified. Previously the check ran after parsing all elements, leaving a partially-populated or reset array slot whenever a math expression like a=(1+2)/3 was mis-detected. The duplicate post-scan block is removed. Agent-Logs-Url: https://github.com/milk-org/milk/sessions/927e716b-3881-4a40-b0d0-4a6f10826c33 Co-authored-by: oguyon <5598733+oguyon@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: oguyon <5598733+oguyon@users.noreply.github.com>
- Add 'dpdigits' CLI command to set/query significant digits (1-17) - Add cli_float_digits global (default 15) in CLIcore_script.h/.c - Replace all hardcoded %.15g with dynamic %.*g using cli_float_digits - Register dpdigits as internal keyword (shell bypass prevention) - Add 6 CLI robustness tests (300/300 pass)
Move the '#' comment check to the very top of CLI_execute_line(), before history expansion, so that $, !, and @ tokens inside comment lines are never processed.
…245) * feat: add assigncheck command and CLI keyword highlighting * feat: add tree-sitter grammar and Neovim integration for milk-cli * fix: variable interpolation and nested variable expansions * fix: guard FPS param accessors against NULL argdata and invalid index Root cause: CLIargs_to_FPSparams_setval dereferenced data.cmd[cmdi].argdata[0] which was NULL when the command used CLICMD_FIELDS_NOPARAM (nbarg=0, no argdata allocated). This caused SIGSEGV on commands like tools.rtprio. Fixes: - CLIcore_checkargs_fps.c: limit loop to min(nbarg, nreg) and check argdata != NULL before accessing it. - fps_paramvalue.c: add fpsi < 0 guard to all 28 Get/Set/GetPtr accessor functions so a missed parameter lookup returns 0/NULL/EXIT_FAILURE instead of accessing parray[-1]. - fps_add_entry.c: read only valueptr[0] for scalar types instead of 4 consecutive values past end of a single variable (OOB read). * docs: add syntax highlighting section to scripting page Document the tree-sitter-milkcli Neovim integration in the MkDocs scripting page, covering quick setup, highlight groups, dynamic module commands, and management scripts. * fix: replace strcpy with strncpy to prevent buffer overflows in CLI - CLIcore_UI_completion.c: readline input could exceed the 1000-byte CLIcmdline buffer (heap overflow) - CLIcore_UI_completion_tab.c: unbounded strcpy of rl_line_buffer into 200-byte stack buffer; also guard strtok return against NULL for all-whitespace input - CLIcore_checkargs_fps.c: two strcpy calls replaced with bounded strncpy for triggerstreamname and FPS_name
…d UI updates (#246) - Add tree-sitter parser and highlight integration for in-CLI syntax coloring (src/cli/CLIcore/treesitter/) - Extend list_variable with enhanced display formatting - Update CLIcore UI builtins and highlight infrastructure - Update CLIcore CMakeLists for treesitter build - Minor fix in linARfilterPred - Update demo.milk example
Linux kernels pass everything after the interpreter in a shebang as a single argument, so '#!/usr/bin/env milk-cli -s' fails because env looks for an executable literally named 'milk-cli -s'. Replace all shebang lines with '#!/usr/bin/env milk-script', which uses the existing standalone script runner binary. Update docs, help text, agent rules, and skill examples accordingly.
- stream_sem.c: all 5 functions that resolve an image name now check for ID == -1 before using dcimg[ID]. Without these guards, any typo in a stream name caused an immediate out-of-bounds segfault. Also added bounds checks to sempost_byID and sempost_excl_byID. - fps_scan.c: added regfree() after regexec() in the FPS list matching loop. Each regcomp() allocates internal buffers that were never freed, leaking memory proportional to fpslistcnt × number of FPS entries on every scan cycle.
…rotect slot allocation - delete_image: moved 'used = 0' to AFTER all munmap/free/close calls. Previously it was the first operation, allowing another thread to reclaim the slot while pointers were still being freed. This matches the safe ordering in ImageStreamIO_destroyIm(). Also converted bare if chains to else-if for datatype dispatch. - image_copy: standardized both copy functions to the canonical sequence: write=1 → memcpy → cnt0++ → write=0 → sempost. Previously, sempost was called while write=1 (waking readers into a spinloop), and cnt0 ordering was inconsistent between the two functions. - image_ID (next_avail_image_ID): moved the preferred-ID fast path inside the same omp critical section as the fallback scan. Two threads requesting the same preferredID could both see used==0 and both claim the slot.
…ecutables Migrate 7 files from signal() to sigaction() with explicit sa_mask/sa_flags. signal() has undefined reset-to-default behavior on some platforms and doesn't support SA_RESTART. Files changed: - milk-fpsCTRL.c: SIGSEGV/SIGBUS/SIGABRT crash handlers; also replaced kill(getpid(),sig) with async-signal-safe raise(sig) - milk-fps-valkey.c: SIGINT/SIGTERM with SA_RESTART - milk-streamCTRL-cli.c: SIGINT/SIGTERM with SA_RESTART - milk-fps-track.c: SIGINT with SA_RESTART - milk-procCTRL-scan.c: SIGINT/SIGTERM with SA_RESTART - milk-perfbench.c: SIGINT/SIGTERM (no SA_RESTART — calls _exit) - CLIcore.c: SIGWINCH with SA_RESTART (readline resize handler)
- milk-procCTRL-scan: replaced subtract-then-check pattern for timer ring buffer indices with proper modulo arithmetic. The old pattern could produce negative intermediate values before the check, which is undefined for array indexing. - milk-fps-track, milk-fps-valkey: use temporary variable for realloc() return. Direct assignment to the same pointer both leaks memory (old pointer lost) and dereferences NULL if realloc fails. Now logs and skips on failure. - milk-fpsCTRL: replaced endwin() in crash handler with raw write() of ANSI reset sequences. endwin() is NOT async-signal-safe — if the signal arrives during an ncurses operation it can deadlock. write() to STDERR_FILENO is guaranteed async-signal-safe by POSIX.
Targeted the highest-risk unbounded string operations in the engine subsystem — all write into fixed-size shared memory buffers or stack arrays where overflow would corrupt SHM or cause stack smashing. - processinfo_setup.c: strcpy → strncpy for source_FUNCTION, source_FILE, description (all 200-byte SHM buffers) - processinfo_WriteMessage.c: strcpy → strncpy for statusmsg - stream_pixmapdecode.c: sprintf → snprintf for pinfoname, pinfodescr, msgstring (all 200-byte stack buffers composed from user-supplied stream names) - fps_scan.c: strcpy(basename()) → strncpy with NUL termination - fps_GetFileName.c: strcpy → strncpy for output filename
…a-src Replace all remaining unsafe string operations in the plugin layer: - linalgebra: GPU_loop_MultMat_execute, SingularValueDecomp, PCAmatch, GPU_SVD_computeControlMatrix, magma_compute_SVDpseudoInverse_SVD, GPU_loop_MultMat_setup - image_gen: image_gen_profiles, image_gen_polygons - ZernikePolyn: ZernikePolyn_fit, ZernikePolyn - linARfilterPred: linARfilterPred, applyPF - info: image_stats, imagemon - image_format: combineHDR, stream_temporal_stats, extract_utr - linopt_imtools: compute_SVDpseudoInverse All sprintf() calls replaced with snprintf(dest, sizeof(dest), ...). All strcpy() calls replaced with snprintf(dest, sizeof(dest), "%s", src). strcpy+strcat pairs collapsed into single snprintf() calls. This completes the codebase-wide string safety audit for src/engine/, src/cli/, src/coremods/, and plugins/milk-extra-src/.
…mods, and remaining plugins Comprehensive bounds-checked string safety remediation across: - src/engine/: libfps (fpsCTRL, fps_scan, fps_tmux, fps_connect, fps_save2disk, fps_FPCONFsetup, fps_print_info, fps_processcmdline_interactive), libfpsseq (fpsseq_script, fpsseq_cmdexec), libprocessinfo (processinfo_SIGexit, processinfo_shm_create, processtools_trigger, procCTRL_TUI, milk-procinfo-list) - src/cli/: CLIcore (UI_completion, UI_completion_tab), libmilkscript (checkargs, help, help_command, signals, UI_execute_debug), streamCTRL (TUI, find_streams) - src/coremods/: COREMOD_arith (execute_arith), COREMOD_iofits (loadmemstream), COREMOD_memory (create_variable, image_copy, image_keyword, image_keyword_addD/addL, image_keyword_list, list_image, logshmim, logshmim_save, saveall, stream_merge, stream_pixmapdecode, stream_updateloop), COREMOD_tools (fileutils, logfunc, mvprocCPUset) - plugins/milk-extra-src/: linalgebra (GPU_SVD_computeControlMatrix, GPU_loop_MultMat_setup, PCAmatch, SingularValueDecomp, magma_compute_SVDpseudoInverse_SVD), linopt_imtools (compute_SVDpseudoInverse) All sprintf() -> snprintf(dest, sizeof(dest), ...). All strcpy() -> snprintf(dest, sizeof(dest), "%s", src) or memcpy(). strcpy+strcat pairs collapsed into single snprintf() calls.
Replace all direct accesses to IMAGE_METADATA.write and cnt0++ across coremods, milk-extra-src, examples, and CLI with the new SHMIM_WRITE_ACQUIRE/RELEASE/LOAD and SHMIM_CNT0_INCREMENT macros from ImageStruct.h. This ensures proper memory fences for cross-process synchronization of shared memory streams, eliminating race conditions from stale reads and compiler reordering.
Point submodule to commit 5176658 which adds the SHMIM_WRITE_*/SHMIM_CNT0_* atomic accessor macros in ImageStruct.h. Required for the milk-side migration.
775205f to
b7872fa
Compare
5bf51ae to
f5e8724
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrate all direct
md->writeandmd[0].writeassignments, and all directmd->cnt0++increments across coremods, milk-extra-src plugins, examples, and CLI to use the newSHMIM_WRITE_*andSHMIM_CNT0_*atomic accessor macros fromImageStruct.h.This is the milk-side companion to the ImageStreamIO PR (milk-org/ImageStreamIO#69) that defines the macros. Together, they eliminate a class of cross-process race conditions from stale reads and compiler reordering of the write flag and frame counter.
Rationale
See milk-org/ImageStreamIO#69 for the full rationale on why atomic accessors are needed for
IMAGE_METADATA.writeandcnt0. This PR performs the mechanical migration of ~95 call sites across 34 files in the milk repository to use the new macros.What changed
Every instance of:
was replaced with:
Diagnostic reads (e.g.,
image_stats.c,imagemon.c) now useSHMIM_WRITE_LOAD()andSHMIM_CNT0_LOAD()for acquire-ordered reads.Backward compatibility
This is a code-only change — no struct layout, ABI, or shared memory format changes. All modifications are source-level substitutions of direct field access with macro calls that operate on the same underlying fields at the same offsets. The compiled binaries produce identical SHM layouts and are fully interoperable with older processes.
Opt-in write blocking
The write flag remains an opt-in cooperative mutex. Readers that want to block until a write completes call
ImageStreamIO_BusywaitForNoWrite(), which now uses atomic loads internally. All writer code paths now useSHMIM_WRITE_ACQUIRE/RELEASEconsistently, ensuring that any reader opting into write blocking will see a fully consistent frame.Readers that do not call
BusywaitForNoWriteare unaffected — they continue to read immediately after semaphore wakeup, as before.Files changed (34 files, +140 −140)
coremods:
COREMOD_memory/: image_copy, image_copy_shm, stream_diff, stream_halfimdiff, stream_merge, stream_paste, stream_pixmapdecode, stream_updateloop, stream_TCP/UDP_receive, im3D_to_stream2D, image_mk_fromCOREMOD_arith/imfunctions.cmilk-extra-src plugins:
linalgebra/: GPU_loop_MultMat_execute, MVMextractModes, cublas_Coeff2Map_LooplinARfilterPred/: applyPF, build_linPF, linARfilterPredimage_basic/streamfeed.cimage_format/: extract_utr, stream_temporal_statsimg_reduce/img_reduce.cinfo/: image_stats, imagemon, stream_monprocexamples:
module templates:
AI Authorship
Prompt Summary
Codebase-wide migration of all direct
IMAGE_METADATA.writeandcnt0field accesses to use the new atomic accessor macros, as part of the cross-process race condition remediation effort.