From 6d4effb684cb01c6cb65b00e76aa65c8666f4237 Mon Sep 17 00:00:00 2001 From: Diego Cesar Anaya Guerrero <306688326+DiegoAnyG@users.noreply.github.com> Date: Mon, 3 Aug 2026 23:30:25 -0600 Subject: [PATCH 1/2] Do directory work in C instead of handing it to a shell fpocket created its output folders with system("mkdir -p ..."), marked the VMD and PyMOL wrappers executable with system("chmod +x ..."), and put its qhull scratch files in a hardcoded /tmp. Three consequences, in increasing order of severity: - the output directory depended on a POSIX shell and coreutils being present; - a filename reached that shell unquoted; - on a system with no /tmp, fopen() returned NULL and load_vvertices() wrote through it, so the tessellation crashed instead of reporting the problem. headers/os_compat.h now holds the three operations that need to know the operating system: m_mkdir/m_mkdir_p, m_make_executable and m_tmpdir. TMPDIR is still honoured first, so an existing setup behaves as before; TEMP and TMP are consulted where they are the convention. The two fopen() calls for the scratch files are checked and report the directory they could not write to. Results are unchanged: on a 2833-atom structure the patched build returns the same 140 pockets with every descriptor identical to the unpatched one, except the Monte Carlo Volume, which already varies by ~1.4% between two runs of the same binary. --- headers/os_compat.h | 189 ++++++++++++++++++++++++++++++++++++++++++++ headers/utils.h | 1 + src/energy.c | 7 +- src/fpout.c | 9 +-- src/voronoi.c | 22 ++++-- src/write_visu.c | 12 +-- 6 files changed, 215 insertions(+), 25 deletions(-) create mode 100644 headers/os_compat.h diff --git a/headers/os_compat.h b/headers/os_compat.h new file mode 100644 index 00000000..7ee7e7dd --- /dev/null +++ b/headers/os_compat.h @@ -0,0 +1,189 @@ +/* + * Copyright <2012> + * Copyright <2013-2018> + +Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: +The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + + */ + + +#ifndef DH_OS_COMPAT +#define DH_OS_COMPAT + +/* ------------------------------ INCLUDES ---------------------------------- */ + +#include +#include +#include +#include + +#ifdef _WIN32 +#include +#else +#include +#endif + +/* ------------------------------ DESCRIPTION ------------------------------- */ +/** + * Directory creation and permissions, which are the only places where fpocket + * needs to know which operating system it runs on. + * + * These used to be done by handing a command to system(): "mkdir -p", "chmod +x". + * That made the output directories depend on a POSIX shell being present, and it + * passed a filename straight to that shell. Doing the same work through the C + * library removes both problems and is what lets fpocket build on Windows, where + * mkdir() takes no mode and chmod +x has no meaning. + */ + +/* ------------------------------- PUBLIC MACROS ---------------------------- */ + +#define M_MAX_MKDIR_PATH 2048 /**< longest path m_mkdir_p will walk */ + +/* ------------------------------ PUBLIC FUNCTIONS -------------------------- */ + +/** + ## FUNCTION: + m_mkdir + + ## SPECIFICATION: + Creates a single directory. An already existing directory is a success: + fpocket rewrites its output folder on every run. + + ## PARAMETRES: + @ const char *path : directory to create + + ## RETURN: + int : 0 on success, -1 otherwise + +*/ +static int m_mkdir(const char *path) +{ + int status; + +#ifdef _WIN32 + status = _mkdir(path); +#else + status = mkdir(path, 0755); +#endif + + if (status != 0 && errno == EEXIST) return 0; + + return status; +} + +/** + ## FUNCTION: + m_mkdir_p + + ## SPECIFICATION: + Creates a directory and every missing parent, as "mkdir -p" did. Both + separators are accepted because fpocket builds its paths with '/' even + when running on Windows. + + ## PARAMETRES: + @ const char *path : directory to create + + ## RETURN: + int : 0 on success, -1 otherwise + +*/ +static int m_mkdir_p(const char *path) +{ + char tmp[M_MAX_MKDIR_PATH]; + size_t len, + i; + + len = strlen(path); + if (len == 0 || len >= sizeof(tmp)) return -1; + + strcpy(tmp, path); + + /* A trailing separator would make the final mkdir act on an empty name. */ + while (len > 1 && (tmp[len-1] == '/' || tmp[len-1] == '\\')) { + tmp[len-1] = '\0'; + len --; + } + + /* i starts at 1 so that a leading '/' is not mistaken for a component. */ + for (i = 1; i < len; i++) { + if (tmp[i] == '/' || tmp[i] == '\\') { + char sep = tmp[i]; + tmp[i] = '\0'; + if (m_mkdir(tmp) != 0) return -1; + tmp[i] = sep; + } + } + + return m_mkdir(tmp); +} + +/** + ## FUNCTION: + m_tmpdir + + ## SPECIFICATION: + Directory for the scratch files handed to qhull. TMPDIR is honoured first + so an existing setup keeps working; Windows names the same thing TEMP or + TMP and has no /tmp, which is why the hardcoded fallback used to leave + fopen() returning NULL and the tessellation writing through it. + + ## PARAMETRES: + void + + ## RETURN: + const char * : an existing directory, without a trailing separator + +*/ +static const char *m_tmpdir(void) +{ + const char *d; + + d = getenv("TMPDIR"); + if (d && *d) return d; + +#ifdef _WIN32 + d = getenv("TEMP"); + if (d && *d) return d; + + d = getenv("TMP"); + if (d && *d) return d; + + return "."; +#else + return "/tmp"; +#endif +} + +/** + ## FUNCTION: + m_make_executable + + ## SPECIFICATION: + Marks a file as executable. fpocket uses it on the shell wrappers it + writes for VMD and PyMOL. Windows decides by extension, so there is + nothing to do there. + + ## PARAMETRES: + @ const char *path : file to mark + + ## RETURN: + int : 0 on success, -1 otherwise + +*/ +static int m_make_executable(const char *path) +{ +#ifdef _WIN32 + (void) path; + return 0; +#else + struct stat st; + + if (stat(path, &st) != 0) return -1; + + return chmod(path, st.st_mode | S_IXUSR | S_IXGRP | S_IXOTH); +#endif +} + +#endif diff --git a/headers/utils.h b/headers/utils.h index fe600458..e23d806c 100644 --- a/headers/utils.h +++ b/headers/utils.h @@ -30,6 +30,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI #endif /* /GSL */ #include "memhandler.h" +#include "os_compat.h" /* ------------------------------- PUBLIC MACROS ---------------------------- */ diff --git a/src/energy.c b/src/energy.c index a5a7c777..3f8ab106 100644 --- a/src/energy.c +++ b/src/energy.c @@ -74,12 +74,9 @@ void calculate_pocket_energy_grids(c_lst_pockets *pockets, s_fparams *params, s_ if (strlen(pdb_path) > 0) sprintf(out_path, "%s/%s_out", pdb_path, pdb_code); else sprintf(out_path, "%s_out", pdb_code); - //sprintf(command, "mkdir %s", out_path) ; - status = mkdir(out_path, 0755); - //status = system(command) ; + status = m_mkdir_p(out_path); sprintf(out_path_pockets, "%s/pockets", out_path); - status = mkdir(out_path_pockets, 0755); - //status = system(command) ; + status = m_mkdir(out_path_pockets); if(status != 0) { return ; } diff --git a/src/fpout.c b/src/fpout.c index 26f501c1..4b6b5dd2 100644 --- a/src/fpout.c +++ b/src/fpout.c @@ -79,8 +79,7 @@ void write_out_fpocket(c_lst_pockets *pockets, s_pdb *pdb, char *pdbname) else sprintf(out_path, "%s_out", pdb_code); - sprintf(command, "mkdir -p %s", out_path); - status = system(command); + status = m_mkdir_p(out_path); if (status != 0) { return; @@ -123,8 +122,7 @@ void write_out_fpocket(c_lst_pockets *pockets, s_pdb *pdb, char *pdbname) sprintf(out_path, "%s_out", pdb_code); sprintf(out_path_tmp, "%s/pockets", out_path); - sprintf(command, "mkdir %s", out_path_tmp); - status = system(command); + status = m_mkdir(out_path_tmp); /*if(status != 0) { return ; }*/ @@ -238,8 +236,7 @@ void write_out_fpocket_DB(c_lst_pockets *pockets, s_pdb *pdb, char *input_name) sprintf(out_path, "%s/%s_out", pdb_path, pdb_code); else sprintf(out_path, "%s_out", pdb_code); - sprintf(command, "mkdir -p %s", out_path); - int status = system(command); + int status = m_mkdir_p(out_path); // Writing full pdb sprintf(pdb_out_path, "%s_out.pdb", out_path); diff --git a/src/voronoi.c b/src/voronoi.c index 2e368648..3b77c315 100644 --- a/src/voronoi.c +++ b/src/voronoi.c @@ -77,10 +77,8 @@ s_lst_vvertice *load_vvertices(s_pdb *pdb, s_fparams *params, float xshift, floa char tmpn1[250] = ""; char tmpn2[250] = ""; - const char *env = getenv("TMPDIR"); + const char *env = m_tmpdir(); pid_t pid = getpid(); - if (!env) - env = "/tmp/"; sprintf(tmpn1, "%s/qvoro_in_fpocket_%d.dat", env, pid); sprintf(tmpn2, "%s/qvoro_out_fpocket_%d.dat", env, pid); @@ -110,6 +108,13 @@ s_lst_vvertice *load_vvertices(s_pdb *pdb, s_fparams *params, float xshift, floa { FILE *ftmp = fopen(tmpn2, "w"); FILE *fvoro = fopen(tmpn1, "w+"); + if (!ftmp || !fvoro) + { + fprintf(stderr, "! Cannot write scratch files in %s. Set TMPDIR to a writable directory.\n", env); + if (ftmp) fclose(ftmp); + if (fvoro) fclose(fvoro); + return NULL; + } /* Write the header for qvoronoi */ fprintf(fvoro, "3 rbox D3\n%d\n", lvvert->n_h_tr); // fprintf(fvoro, "3 rbox D3\n%d\n", 100) ; @@ -1244,13 +1249,18 @@ float get_convex_hull_volume(s_vvertice **verts, int nvert) if (nvert < 10) return (0.0); - const char *env = getenv("TMPDIR"); - if (!env) - env = "/tmp/"; + const char *env = m_tmpdir(); sprintf(tmpn1, "%s/qhull_in_fpocket_%d.dat", env, pid); sprintf(tmpn2, "%s/qhull_out_fpocket_%d.dat", env, pid); FILE *ftmp = fopen(tmpn2, "w"); FILE *fvoro = fopen(tmpn1, "w+"); + if (!ftmp || !fvoro) + { + fprintf(stderr, "! Cannot write scratch files in %s. Set TMPDIR to a writable directory.\n", env); + if (ftmp) fclose(ftmp); + if (fvoro) fclose(fvoro); + return (0.0); + } /* Write the header for qvoronoi */ fprintf(fvoro, "3 rbox D3\n%d\n", nvert); diff --git a/src/write_visu.c b/src/write_visu.c index 643659f2..7b0db79a 100644 --- a/src/write_visu.c +++ b/src/write_visu.c @@ -111,8 +111,7 @@ void write_vmd(char *pdb_name, char *pdb_out_name) fclose(f); /* Make tcl script executable, and Write tcl script */ - sprintf(sys_cmd, "chmod +x %s", fout); - status = system(sys_cmd); + status = m_make_executable(fout); fprintf(f_tcl, "proc highlighting { colorId representation id selection } {\n"); fprintf(f_tcl, " puts \"highlighting $id\"\n"); @@ -181,8 +180,7 @@ void write_vmd_mmcif(char *pdb_name, char *pdb_out_name) fclose(f); /* Make tcl script executable, and Write tcl script */ - sprintf(sys_cmd, "chmod +x %s", fout); - status = system(sys_cmd); + status = m_make_executable(fout); fprintf(f_tcl, "proc highlighting { colorId representation id selection } {\n"); fprintf(f_tcl, " puts \"highlighting $id\"\n"); @@ -262,8 +260,7 @@ void write_pymol(char *pdb_name, char *pdb_out_name) fflush(f); fclose(f); - sprintf(sys_cmd, "chmod +x %s", fout); - status = system(sys_cmd); + status = m_make_executable(fout); /* Write pml script */ fprintf(f_pml, "from pymol import cmd,stored\n"); fprintf(f_pml, "load %s\n", pdb_out_name); @@ -318,8 +315,7 @@ void write_pymol_mmcif(char *pdb_name, char *pdb_out_name) fflush(f); fclose(f); - sprintf(sys_cmd, "chmod +x %s", fout); - status = system(sys_cmd); + status = m_make_executable(fout); /* Write pml script */ fprintf(f_pml, "from pymol import cmd,stored\n"); fprintf(f_pml, "load %s\n", pdb_out_name); From 8e7bd396400b31cbf95c009a830ddc1c8c2259c3 Mon Sep 17 00:00:00 2001 From: Diego Cesar Anaya Guerrero <306688326+DiegoAnyG@users.noreply.github.com> Date: Mon, 3 Aug 2026 23:30:25 -0600 Subject: [PATCH 2/2] Allow building without the molfile plugins The vendored VMD libmolfile_plugin is only used for mmCIF input (read_mmcif.c) and Amber topologies (topology.c), both reached from two call sites in fpmain.c. Where that library is unusable, -DM_NO_MOLFILE leaves those two formats out and builds the rest, which is what makes a Windows binary possible today: the shipped WIN64 library is compiled with MSVC, so its objects pull in __security_cookie and __GSHandlerCheck that MinGW cannot resolve, and it has no pdbx plugin at all, so mmCIF could not link even with a matching compiler. PDB input, the Voronoi tessellation and every descriptor are unaffected. open_file_format() also initialises its return value, which was left uninitialised when the input matched neither extension. --- src/fpmain.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/fpmain.c b/src/fpmain.c index 8abc67d3..645b39ab 100644 --- a/src/fpmain.c +++ b/src/fpmain.c @@ -153,7 +153,11 @@ void process_pdb(char *pdbname, s_fparams *params) if (params->topology_path[0] != 0) { +#ifdef M_NO_MOLFILE + fprintf(stderr, "! This build ignores --topology_file; it needs the molfile plugins.\n"); +#else read_topology(params->topology_path, pdb); +#endif } if (pdb) @@ -230,9 +234,13 @@ void process_pdb(char *pdbname, s_fparams *params) s_pdb *open_file_format(char *fpath, const char *ligan, const int keep_lig, int model_number, s_fparams *par) { - s_pdb *pdb; + s_pdb *pdb = NULL; if (strstr(par->pdb_path, ".cif")) /*strstr finds the substring and here we search for the file extension we want */ +#ifdef M_NO_MOLFILE + fprintf(stderr, "! This build reads PDB only; mmCIF needs the molfile plugins.\n"); +#else pdb = open_mmcif(fpath, NULL, keep_lig, par->model_number, par); +#endif else if (strstr(par->pdb_path, ".pdb")) pdb = rpdb_open(fpath, NULL, keep_lig, par->model_number, par); @@ -244,7 +252,9 @@ void read_file_format(s_pdb *pdb, const char *ligan, const int keep_lig, int mod if (strstr(par->pdb_path, ".cif")) { /*strstr finds the substring and here we search for the file extension we want */ +#ifndef M_NO_MOLFILE read_mmcif(pdb, NULL, keep_lig, par->model_number, par); +#endif } else if (strstr(par->pdb_path, ".pdb")) rpdb_read(pdb, NULL, keep_lig, par->model_number, par);