From 6be6a39945986e97537d971d6341b9345718d199 Mon Sep 17 00:00:00 2001 From: David de Hilster Date: Wed, 14 Oct 2026 23:07:56 -0400 Subject: [PATCH] File and directory helpers on std::filesystem; mkdir() without the shell (#323) (4.2.31) Stage 1 of #323. - NLP++ mkdir() ran a shell "mkdir PATH": a path with spaces, or on Windows with forward slashes, made nothing; a path with & or ; ran a command; and it always returned 1. It now uses create_directories and returns whether the directory exists. - lite dir_exists/make_dir/rm_dir were #ifndef LINUX: on Linux and macOS dir_exists was always true and make_dir/rm_dir did nothing. - kbdumptree() ran "cat" through the shell on Linux and macOS; all platforms now concatenate in C++, after closing the files being read. - prim make_dir/rm_dir, file_size, file_exists (a directory no longer "exists" as a file on Linux), and nlp_engine stat/mkdir use std::filesystem. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/build-linux.yml | 34 +++++++++++++ .github/workflows/build-windows.yml | 33 ++++++++++++ .github/workflows/tests/fs-ops/input/text.txt | 1 + .../workflows/tests/fs-ops/spec/analyzer.seq | 2 + .github/workflows/tests/fs-ops/spec/ops.nlp | 22 ++++++++ .github/workflows/tests/fs-ops/tmp/attr.kb | 9 ++++ .github/workflows/tests/fs-ops/tmp/hier.kb | 2 + .github/workflows/tests/fs-ops/tmp/phr.kb | 3 ++ .github/workflows/tests/fs-ops/tmp/word.kb | 0 cs/libconsh/cg.cpp | 17 +++---- cs/libprim/dir.cpp | 24 +++------ lite/fn.cpp | 13 +++-- lite/fnrun.cpp | 13 +++-- lite/io.cpp | 10 ++-- lite/mach.cpp | 51 ++++++++----------- lite/nlp_engine.cpp | 16 +++--- lite/std.cpp | 22 +++----- nlp/main.cpp | 2 +- 18 files changed, 179 insertions(+), 95 deletions(-) create mode 100644 .github/workflows/tests/fs-ops/input/text.txt create mode 100644 .github/workflows/tests/fs-ops/spec/analyzer.seq create mode 100644 .github/workflows/tests/fs-ops/spec/ops.nlp create mode 100644 .github/workflows/tests/fs-ops/tmp/attr.kb create mode 100644 .github/workflows/tests/fs-ops/tmp/hier.kb create mode 100644 .github/workflows/tests/fs-ops/tmp/phr.kb create mode 100644 .github/workflows/tests/fs-ops/tmp/word.kb diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml index 874a4916..54f30d6b 100644 --- a/.github/workflows/build-linux.yml +++ b/.github/workflows/build-linux.yml @@ -1154,6 +1154,40 @@ jobs: fi echo "[dict-shape] dictionary join, split, _name lines and bare entries" + - name: mkdir() and kbdumptree() without the shell (#323) (Non-20.04) + if: matrix.id != 'docker-ubuntu-20.04' + shell: bash + run: | + set -eo pipefail + # mkdir() was a shell "mkdir PATH": a path with spaces (or, on + # Windows, forward slashes) made nothing, a path with & ran a + # command, and it always returned 1. kbdumptree() ran "cat" on + # Linux and macOS. Both use std::filesystem / C++ now (#323). + FIX=.github/workflows/tests/fs-ops + rm -rf "$FIX/logs" "$FIX/input/text.txt_log" "$FIX/output" "$FIX/made" pwned_323.txt + mkdir -p "$FIX/logs" + ./bin/nlp -ANA "$FIX" -WORK ./ "$FIX/input/text.txt" + OUT="$FIX/input/text.txt_log/output.txt" + if [ ! -f "$OUT" ]; then + echo "FAIL(fs-ops): no output.txt -- the analyzer did not build"; exit 1 + fi + got=$(tr -d '\r' < "$OUT") + want=$(printf '%s\n' 'plain 1' 'spaces 1' 'nested 1' 'again 1' 'dump 1') + if [ "$got" != "$want" ]; then + echo "FAIL(fs-ops): output.txt was:"; echo "$got"; echo "wanted:"; echo "$want"; exit 1 + fi + for d in plain "with spaces" a/b/c; do + [ -d "$FIX/made/$d" ] || { echo "FAIL(fs-ops): mkdir() did not make $d"; exit 1; } + done + if [ -e pwned_323.txt ]; then + echo "FAIL(fs-ops): mkdir() ran a shell command from its path"; exit 1 + fi + DUMP="$FIX/made/dump.kb" + grep -q 'add hier "concept" "fsTest" "child"' "$DUMP" || { echo "FAIL(fs-ops): dump.kb lacks the child"; cat "$DUMP"; exit 1; } + grep -q '^"blue"' "$DUMP" || { echo "FAIL(fs-ops): dump.kb lacks the attribute"; cat "$DUMP"; exit 1; } + [ "$(tr -d '\r' < "$DUMP" | grep -v '^$' | tail -1)" = quit ] || { echo "FAIL(fs-ops): dump.kb does not end in quit"; cat "$DUMP"; exit 1; } + echo "[fs-ops] mkdir() and kbdumptree() work without the shell" + - name: factorial() up to 20! (Non-20.04) if: matrix.id != 'docker-ubuntu-20.04' shell: bash diff --git a/.github/workflows/build-windows.yml b/.github/workflows/build-windows.yml index f9411799..d5a6d84c 100644 --- a/.github/workflows/build-windows.yml +++ b/.github/workflows/build-windows.yml @@ -1048,6 +1048,39 @@ jobs: fi echo "[dict-shape] dictionary join, split, _name lines and bare entries" + - name: mkdir() and kbdumptree() without the shell (#323) + shell: bash + run: | + set -eo pipefail + # mkdir() was a shell "mkdir PATH": a path with spaces (or, on + # Windows, forward slashes) made nothing, a path with & ran a + # command, and it always returned 1. kbdumptree() ran "cat" on + # Linux and macOS. Both use std::filesystem / C++ now (#323). + FIX=.github/workflows/tests/fs-ops + rm -rf "$FIX/logs" "$FIX/input/text.txt_log" "$FIX/output" "$FIX/made" pwned_323.txt + mkdir -p "$FIX/logs" + ./bin/Release/nlp.exe -ANA "$FIX" -WORK ./ "$FIX/input/text.txt" + OUT="$FIX/input/text.txt_log/output.txt" + if [ ! -f "$OUT" ]; then + echo "FAIL(fs-ops): no output.txt -- the analyzer did not build"; exit 1 + fi + got=$(tr -d '\r' < "$OUT") + want=$(printf '%s\n' 'plain 1' 'spaces 1' 'nested 1' 'again 1' 'dump 1') + if [ "$got" != "$want" ]; then + echo "FAIL(fs-ops): output.txt was:"; echo "$got"; echo "wanted:"; echo "$want"; exit 1 + fi + for d in plain "with spaces" a/b/c; do + [ -d "$FIX/made/$d" ] || { echo "FAIL(fs-ops): mkdir() did not make $d"; exit 1; } + done + if [ -e pwned_323.txt ]; then + echo "FAIL(fs-ops): mkdir() ran a shell command from its path"; exit 1 + fi + DUMP="$FIX/made/dump.kb" + grep -q 'add hier "concept" "fsTest" "child"' "$DUMP" || { echo "FAIL(fs-ops): dump.kb lacks the child"; cat "$DUMP"; exit 1; } + grep -q '^"blue"' "$DUMP" || { echo "FAIL(fs-ops): dump.kb lacks the attribute"; cat "$DUMP"; exit 1; } + [ "$(tr -d '\r' < "$DUMP" | grep -v '^$' | tail -1)" = quit ] || { echo "FAIL(fs-ops): dump.kb does not end in quit"; cat "$DUMP"; exit 1; } + echo "[fs-ops] mkdir() and kbdumptree() work without the shell" + - name: factorial() up to 20! shell: bash run: | diff --git a/.github/workflows/tests/fs-ops/input/text.txt b/.github/workflows/tests/fs-ops/input/text.txt new file mode 100644 index 00000000..587be6b4 --- /dev/null +++ b/.github/workflows/tests/fs-ops/input/text.txt @@ -0,0 +1 @@ +x diff --git a/.github/workflows/tests/fs-ops/spec/analyzer.seq b/.github/workflows/tests/fs-ops/spec/analyzer.seq new file mode 100644 index 00000000..3765024c --- /dev/null +++ b/.github/workflows/tests/fs-ops/spec/analyzer.seq @@ -0,0 +1,2 @@ +tokenize nil # +nlp ops # diff --git a/.github/workflows/tests/fs-ops/spec/ops.nlp b/.github/workflows/tests/fs-ops/spec/ops.nlp new file mode 100644 index 00000000..f99e25be --- /dev/null +++ b/.github/workflows/tests/fs-ops/spec/ops.nlp @@ -0,0 +1,22 @@ +############################################### +# FILE: ops.nlp +# SUBJ: mkdir() and kbdumptree() (#323). +# NOTE: mkdir() was a shell "mkdir PATH": paths with spaces or +# forward slashes (on Windows) made nothing, a path with & ran +# a command, and it always returned 1. kbdumptree() ran "cat" +# on Linux and macOS. Expected output is in the CI step. +############################################### + +@CODE +L("base") = G("$apppath") + "/made"; +"output.txt" << "plain " << mkdir(L("base") + "/plain") << "\n"; +"output.txt" << "spaces " << mkdir(L("base") + "/with spaces") << "\n"; +"output.txt" << "nested " << mkdir(L("base") + "/a/b/c") << "\n"; +"output.txt" << "again " << mkdir(L("base") + "/plain") << "\n"; +# Must not run "echo": the CI step checks no pwned_323.txt appears. +mkdir(L("base") + "/x & echo pwned > pwned_323.txt"); + +L("con") = makeconcept(findroot(), "fsTest"); +addstrval(makeconcept(L("con"), "child"), "color", "blue"); +"output.txt" << "dump " << kbdumptree(L("con"), L("base") + "/dump.kb") << "\n"; +@@CODE diff --git a/.github/workflows/tests/fs-ops/tmp/attr.kb b/.github/workflows/tests/fs-ops/tmp/attr.kb new file mode 100644 index 00000000..6b6ee48c --- /dev/null +++ b/.github/workflows/tests/fs-ops/tmp/attr.kb @@ -0,0 +1,9 @@ +ind attr +"concept" "fsTest" "child" +0 +"concept" "sys" "dict" "a" "color" +pst +"blue" +end ind + + diff --git a/.github/workflows/tests/fs-ops/tmp/hier.kb b/.github/workflows/tests/fs-ops/tmp/hier.kb new file mode 100644 index 00000000..ae366a29 --- /dev/null +++ b/.github/workflows/tests/fs-ops/tmp/hier.kb @@ -0,0 +1,2 @@ +add hier "concept" "fsTest" +add hier "concept" "fsTest" "child" diff --git a/.github/workflows/tests/fs-ops/tmp/phr.kb b/.github/workflows/tests/fs-ops/tmp/phr.kb new file mode 100644 index 00000000..3b3c7fc6 --- /dev/null +++ b/.github/workflows/tests/fs-ops/tmp/phr.kb @@ -0,0 +1,3 @@ + +quit + diff --git a/.github/workflows/tests/fs-ops/tmp/word.kb b/.github/workflows/tests/fs-ops/tmp/word.kb new file mode 100644 index 00000000..e69de29b diff --git a/cs/libconsh/cg.cpp b/cs/libconsh/cg.cpp index 65c43728..5fe3e17f 100644 --- a/cs/libconsh/cg.cpp +++ b/cs/libconsh/cg.cpp @@ -3066,9 +3066,12 @@ delete f_attr; // 08/07/03 AM. // Terminate the last command file. f_phr << std::endl << _T("quit") << std::endl << std::endl; -// TODO: CATENATE THE COMMAND FILES TO THE OUTPUT FILE. -#ifndef LINUX -// Do the catenate in C++. +// Catenate the command files to the output file, in C++ on every +// platform: Linux and macOS ran "cat", through the shell (#323). Flush +// the files still open first, or their last lines may be missing. +f_hier.close(); +f_word.close(); +f_phr.close(); _TCHAR buf[MAXMSG]; std::_t_ofstream *f_dump = new std::_t_ofstream(TCHAR2A(file)); // Final output file. std::_t_ifstream *f_in = new std::_t_ifstream(TCHAR2A(o_hier)); @@ -3096,14 +3099,6 @@ delete f_in; delete f_dump; -#else -_TCHAR cmd[MAXPATH*9]; -_TCHAR *cp = _T("cat"); -_stprintf(cmd, _T("%s %s %s %s %s > %s"), - cp, o_hier, o_word, o_attr, o_phr, file); -int systemRet = _tsystem(cmd); -#endif - *cgerr << _T("[Done with modular kb dump.]") << std::endl; return true; } diff --git a/cs/libprim/dir.cpp b/cs/libprim/dir.cpp index 4af8437c..dfad88b1 100644 --- a/cs/libprim/dir.cpp +++ b/cs/libprim/dir.cpp @@ -10,6 +10,7 @@ All rights reserved. * *******************************************************************************/ #include "StdAfx.h" +#include // #323. #ifdef LINUX #include #include @@ -34,14 +35,8 @@ make_dir(_TCHAR *dir) { if (!dir || !*dir) return false; -#ifdef LINUX -if (mkdir(dir, 0755) == 0) - return true; -#else -if (_tmkdir(dir) == 0) - return true; -#endif -return false; +std::error_code ec; +return std::filesystem::create_directory(std::filesystem::path(dir), ec); // #323. } @@ -57,12 +52,9 @@ rm_dir(_TCHAR *dir) { if (!dir || !*dir) return false; -#ifdef LINUX -if (rmdir(dir) == 0) - return true; -#else -if (_trmdir(dir) == 0) - return true; -#endif -return false; +std::error_code ec; +std::filesystem::path p(dir); +if (!std::filesystem::is_directory(p, ec)) + return false; +return std::filesystem::remove(p, ec); // Only if empty. // #323. } diff --git a/lite/fn.cpp b/lite/fn.cpp index fe2ee47a..67a4c5d9 100644 --- a/lite/fn.cpp +++ b/lite/fn.cpp @@ -12,6 +12,7 @@ All rights reserved. *******************************************************************************/ #include "StdAfx.h" +#include // #323. #include //MAXINT,MAXLONG,MAX_INT,MAX_LONG // 01/20/00 AM. #include // For char handling. // 01/06/03 AM. #include // For math, log10. // 04/29/04 AM. @@ -11284,10 +11285,14 @@ ok = (_tsystem(cmd) ? false : true); #endif #endif -_TCHAR cmd[2048]; // 09/08/28 AM. -_stprintf(cmd,_T("mkdir %s"), str); // 09/08/28 AM. -run_silent(cmd); // system() w/o DOC CMD PROMPT POPUP. // 09/28/08 AM. -ok = true; // 09/28/08 AM. +// Was a shell "mkdir STR": a path with spaces failed, one with & or ; +// ran a command, and it always reported success (#323). +{ +std::error_code ec; +std::filesystem::path dp(str); +std::filesystem::create_directories(dp, ec); +ok = std::filesystem::is_directory(dp, ec); +} if (!ok) { diff --git a/lite/fnrun.cpp b/lite/fnrun.cpp index fded9492..b6887909 100644 --- a/lite/fnrun.cpp +++ b/lite/fnrun.cpp @@ -14,6 +14,7 @@ All rights reserved. #include "StdAfx.h" +#include // #323. #ifdef LINUX #ifdef _ODBC #include @@ -17641,10 +17642,14 @@ ok = (_tsystem(cmd) ? false : true); #endif #endif -_TCHAR cmd[2048]; // 09/28/08 AM. -_stprintf(cmd,_T("mkdir %s"), str); // 09/28/08 AN. -run_silent(cmd); // system() w/o DOC CMD PROMPT POPUP. // 09/28/08 AM. -ok = true; // 09/28/08 AM. +// Was a shell "mkdir STR": a path with spaces failed, one with & or ; +// ran a command, and it always reported success (#323). +{ +std::error_code ec; +std::filesystem::path dp(str); +std::filesystem::create_directories(dp, ec); +ok = std::filesystem::is_directory(dp, ec); +} if (!ok) { diff --git a/lite/io.cpp b/lite/io.cpp index 22bb7443..5a41294b 100644 --- a/lite/io.cpp +++ b/lite/io.cpp @@ -11,6 +11,7 @@ All rights reserved. *******************************************************************************/ #include "StdAfx.h" +#include // #323. #include "machine.h" // 03/09/00 AM. #include "u_out.h" // 01/19/06 AM. #include "prim/libprim.h" // 01/18/06 AM. @@ -91,9 +92,12 @@ for (;;) // 12/18/01 AM. bool file_exists(const _TCHAR *iname) { -//ifstream inFile(iname, std::ios::in | std::ios::nocreate); -std::_t_ifstream inFile(CTCHAR2CA(iname), std::ios::in); -return (inFile ? true : false); +// A file, not a directory: opening a directory for reading succeeds on +// Linux and macOS, so a directory used to "exist" there (#323). +if (!iname || !*iname) + return false; +std::error_code ec; +return std::filesystem::is_regular_file(std::filesystem::path(iname), ec); } diff --git a/lite/mach.cpp b/lite/mach.cpp index 2b2de0da..14a73e88 100644 --- a/lite/mach.cpp +++ b/lite/mach.cpp @@ -11,6 +11,7 @@ All rights reserved. *******************************************************************************/ #include "StdAfx.h" +#include // #323. #include #ifndef LINUX #include @@ -265,25 +266,11 @@ void _tmain( void ){ LITE_API bool dir_exists(_TCHAR *name) { -_TCHAR cwd[MAXSTR]; - -/* Get the current working directory: */ -#ifndef LINUX -if(_tgetcwd( cwd, MAXSTR) == NULL) - { - std::_t_strstream gerrStr; - gerrStr << _T("[dir_exists: Couldn't get working dir.]") << std::ends; - errOut(&gerrStr,false); - return false; - } - -if(_tchdir(name)) +// Was a chdir() there and back, and on Linux and macOS always true (#323). +if (!name || !*name) return false; - -// Was able to change to it. Now change back. -_tchdir(cwd); -#endif -return true; +std::error_code ec; +return std::filesystem::is_directory(std::filesystem::path(name), ec); } @@ -297,11 +284,11 @@ return true; LITE_API bool make_dir(_TCHAR *name) { -#ifndef LINUX -if(_tmkdir(name) == 0) - return true; -#endif -return false; +// On Linux and macOS this used to do nothing and return false (#323). +if (!name || !*name) + return false; +std::error_code ec; +return std::filesystem::create_directory(std::filesystem::path(name), ec); } @@ -315,11 +302,14 @@ return false; LITE_API bool rm_dir(_TCHAR *name) { -#ifndef LINUX -if(_trmdir(name) == 0) - return true; -#endif -return false; +// On Linux and macOS this used to do nothing and return false (#323). +if (!name || !*name) + return false; +std::error_code ec; +std::filesystem::path p(name); +if (!std::filesystem::is_directory(p, ec)) + return false; +return std::filesystem::remove(p, ec); // Only if empty. } @@ -335,9 +325,8 @@ LITE_API bool remove_path(_TCHAR *name) { if (!safe_dir(name)) return false; -if(_tremove(name) == 0) - return true; -return false; +std::error_code ec; +return std::filesystem::remove(std::filesystem::path(name), ec); } /******************************************** diff --git a/lite/nlp_engine.cpp b/lite/nlp_engine.cpp index 04208ef2..b0fe6dce 100644 --- a/lite/nlp_engine.cpp +++ b/lite/nlp_engine.cpp @@ -229,7 +229,7 @@ int NLP_ENGINE::init( return -1; } - struct stat st; + std::error_code ec; // #323. if (!bprintf(m_anadir,_T("%s"),analyzer)) { std::_t_cerr << _T("[Error: analyzer path too long: ") << analyzer << _T("]") << std::endl; m_analyzer = 0; @@ -237,7 +237,7 @@ int NLP_ENGINE::init( } if (m_workingFolder[0] != '\0') { - if (stat(analyzer,&st) != 0) { + if (!std::filesystem::exists(std::filesystem::path(analyzer), ec)) { if (!bprintf(m_anadir, _T("%s%sanalyzers%s%s"),m_workingFolder,DIR_STR,DIR_STR,analyzer)) { std::_t_cerr << _T("[Error: analyzer path too long: ") << analyzer << _T("]") << std::endl; m_analyzer = 0; @@ -251,7 +251,7 @@ int NLP_ENGINE::init( } m_analyzer = 0; - if (stat(m_anadir,&st) != 0) { + if (!std::filesystem::exists(std::filesystem::path(m_anadir), ec)) { // A failure, not a success: returning 0 here let analyze() go on and // a compile run of a missing analyzer report success (#484). std::_t_cerr << _T("[analyzer directory not found: ") << m_anadir << _T("]") << std::endl; @@ -801,13 +801,9 @@ int NLP_ENGINE::close(_TCHAR *analyzer) } int NLP_ENGINE::createDir(_TCHAR *dirPath) { - struct stat st; - if (stat(dirPath,&st) != 0) { -#ifdef LINUX - mkdir(dirPath, 0755); -#else - CreateDirectory(dirPath,NULL); -#endif + std::error_code ec; // #323. + if (!std::filesystem::exists(std::filesystem::path(dirPath), ec)) { + std::filesystem::create_directories(std::filesystem::path(dirPath), ec); std::_t_cerr << _T("[Creating output directory: ") << dirPath << _T("]") << std::endl; } return 0; diff --git a/lite/std.cpp b/lite/std.cpp index a3053225..f821ac17 100644 --- a/lite/std.cpp +++ b/lite/std.cpp @@ -12,6 +12,7 @@ All rights reserved. #include "StdAfx.h" +#include // #323. #include #include #include @@ -32,21 +33,12 @@ long file_size(_TCHAR *fname) { long len=0; -/* Open a file */ -// CHANGING THIS TO A BETTER MEASURE. // 07/18/00 AM. -#ifdef LINUX -std::_t_ifstream x; // 10/26/06 AM. -x.open(fname,std::ios::binary); // 10/26/06 AM. -x.seekg(0,std::ios::end); // 10/26/06 AM. -len = x.tellg(); // 10/26/06 AM. -x.close(); // 10/26/06 AM. -#else -struct _stat buf; -int result = _tstat(fname, &buf); -if (result == 0) { - len = buf.st_size; -} -#endif +// One way on every platform (#323). Was a seek to the end of an +// opened file on Linux and _tstat elsewhere. +std::error_code ec; +std::uintmax_t sz = std::filesystem::file_size(std::filesystem::path(fname), ec); +if (!ec) + len = (long) sz; #ifdef VCPPONLY_ //int fh; diff --git a/nlp/main.cpp b/nlp/main.cpp index 10c3a52d..7682fba7 100644 --- a/nlp/main.cpp +++ b/nlp/main.cpp @@ -15,7 +15,7 @@ All rights reserved. #include "lite/nlp_engine.h" #include "version.h" -#define NLP_ENGINE_VERSION "4.2.30" +#define NLP_ENGINE_VERSION "4.2.31" bool cmdReadArgs(int, _TCHAR *argv[], _TCHAR *&, _TCHAR *&, _TCHAR *&, _TCHAR *&, bool &, bool &, bool &, bool &, bool &, bool &, int &, bool &); void cmdHelpargs(_TCHAR *);