From 80ff3be5a2aaba9de98eb9360f88231aa21da6cb Mon Sep 17 00:00:00 2001
From: "Rob Swindell (on Debian Linux)" <rob@synchro.net>
Date: Thu, 20 Feb 2025 17:37:00 -0800
Subject: [PATCH] Add optional error/result outparam to highlevel filedat API
 functions

- loadfile
- addfile
- removefile
- updatefile

As discovered via ftp server "error updating" file log message, no details
could be logged since these functions just returned a simple bool. So, we
now support passing an optional output parameter to capture the result/error
value to be displayed or logged when these functions return false.

Unrelated fixes to start_batch_download():
When passing a list of files on the command to the file transfer protocol
driver (e.g. sz), if any filename contains spaces, that filename must be
quoted or else sz will fail with error 'sz: can read only one file from stdin'.
Also, the trailing whitespace from the list of filenames must be removed or
else sz will report errors 'sz: cannot open : No such file or directory'
and 'sz: Transfer incomplete'.
---
 src/sbbs3/bat_xfer.cpp | 21 ++++++++++++++----
 src/sbbs3/execfile.cpp |  2 +-
 src/sbbs3/file.cpp     | 11 +++++-----
 src/sbbs3/filedat.c    | 50 ++++++++++++++++++++++++++++--------------
 src/sbbs3/filedat.h    |  8 +++----
 src/sbbs3/ftpsrvr.c    | 24 ++++++++++----------
 src/sbbs3/listfile.cpp |  2 +-
 src/sbbs3/qwk.cpp      |  2 +-
 src/sbbs3/upload.cpp   |  2 +-
 src/sbbs3/userdat.c    |  6 ++---
 src/sbbs3/websrvr.c    |  2 +-
 11 files changed, 81 insertions(+), 49 deletions(-)

diff --git a/src/sbbs3/bat_xfer.cpp b/src/sbbs3/bat_xfer.cpp
index ab20b2e051..f8c9a6b56e 100644
--- a/src/sbbs3/bat_xfer.cpp
+++ b/src/sbbs3/bat_xfer.cpp
@@ -227,6 +227,16 @@ bool sbbs_t::batch_upload()
 	return result;
 }
 
+/****************************************************************************/
+/****************************************************************************/
+static const char* quoted_string(const char* str, char* buf, size_t maxlen)
+{
+	if (strchr(str, ' ') == NULL)
+		return str;
+	safe_snprintf(buf, maxlen, "\"%s\"", str);
+	return buf;
+}
+
 /****************************************************************************/
 /* Download files from batch queue                                          */
 /****************************************************************************/
@@ -323,12 +333,14 @@ bool sbbs_t::start_batch_download()
 		totalsize += getfilesize(&cfg, &f);
 		if (!(cfg.dir[f.dir]->misc & DIR_TFREE))
 			totaltime += gettimetodl(&cfg, &f, cur_cps);
-		SAFECAT(list, getfilepath(&cfg, &f, path));
+		char qpath[MAX_PATH + 1];
+		SAFECAT(list, quoted_string(getfilepath(&cfg, &f, path), qpath, sizeof qpath));
 		SAFECAT(list, " ");
 		smb_freefilemem(&f);
 	}
 	iniFreeStringList(ini);
 	iniFreeStringList(filenames);
+	truncsp(list);
 
 	if (!(useron.exempt & FLAG('T')) && !SYSOP && totaltime > (int64_t)timeleft) {
 		bputs(text[NotEnoughTimeToDl]);
@@ -429,6 +441,7 @@ bool sbbs_t::start_batch_download()
 bool sbbs_t::create_batchdn_lst(bool native)
 {
 	char  path[MAX_PATH + 1];
+	int   errval;
 
 	SAFEPRINTF(path, "%sBATCHDN.LST", cfg.node_dir);
 	FILE* fp = fopen(path, "wb");
@@ -452,8 +465,8 @@ bool sbbs_t::create_batchdn_lst(bool native)
 			batch_file_remove(&cfg, useron.number, XFER_BATCH_DOWNLOAD, filename);
 			continue;
 		}
-		if (!loadfile(&cfg, f.dir, filename, &f, file_detail_index)) {
-			errormsg(WHERE, "loading file", filename, i);
+		if (!loadfile(&cfg, f.dir, filename, &f, file_detail_index, &errval)) {
+			errormsg(WHERE, "loading file", filename, errval);
 			batch_file_remove(&cfg, useron.number, XFER_BATCH_DOWNLOAD, filename);
 			continue;
 		}
@@ -673,7 +686,7 @@ void sbbs_t::batch_add_list(char *list)
 					outchar('.');
 					if (k && !(k % 5))
 						bputs("\b\b\b\b\b     \b\b\b\b\b");
-					if (loadfile(&cfg, usrdir[i][j], str, &f, file_detail_normal)) {
+					if (loadfile(&cfg, usrdir[i][j], str, &f, file_detail_normal, NULL)) {
 						if (fexist(getfilepath(&cfg, &f, path)))
 							addtobatdl(&f);
 						else
diff --git a/src/sbbs3/execfile.cpp b/src/sbbs3/execfile.cpp
index 9b642701f3..0eaa0012b1 100644
--- a/src/sbbs3/execfile.cpp
+++ b/src/sbbs3/execfile.cpp
@@ -392,7 +392,7 @@ int sbbs_t::exec_file(csi_t *csi)
 				for (y = 0; y < usrdirs[x]; y++) {
 					if (msgabort())
 						return 0;
-					if (loadfile(&cfg, usrdir[x][y], csi->str, &f, file_detail_normal)) {
+					if (loadfile(&cfg, usrdir[x][y], csi->str, &f, file_detail_normal, NULL)) {
 						addtobatdl(&f);
 						smb_freefilemem(&f);
 						csi->logic = LOGIC_TRUE;
diff --git a/src/sbbs3/file.cpp b/src/sbbs3/file.cpp
index 9094547058..19bc9555f4 100644
--- a/src/sbbs3/file.cpp
+++ b/src/sbbs3/file.cpp
@@ -242,10 +242,11 @@ bool sbbs_t::movefile(smb_t* smb, file_t* f, int newdir)
 
 	newfile.dir = newdir;
 	newfile.dfield = NULL; // addfile() ends up realloc'ing dfield (in smb_addmsg)
-	bool result = addfile(&cfg, &newfile, newfile.extdesc, newfile.auxdata, /* client: */ NULL);
+	int errval;
+	bool result = addfile(&cfg, &newfile, newfile.extdesc, newfile.auxdata, /* client: */ NULL, &errval);
 	free(newfile.dfield);
 	if (!result) {
-		errormsg(WHERE, "adding file", f->name, newfile.dir);
+		errormsg(WHERE, "adding file", f->name, errval);
 		return false;
 	}
 	if (!removefile(smb, f)) // Use ::removefile() here instead?
@@ -297,7 +298,7 @@ bool sbbs_t::editfilename(file_t* f)
 	}
 	bprintf(text[FileRenamed], path, tmp);
 	smb_new_hfield_str(f, SMB_FILENAME, str);
-	return updatefile(&cfg, f);
+	return updatefile(&cfg, f, NULL);
 }
 
 bool sbbs_t::editfiledesc(file_t* f)
@@ -313,7 +314,7 @@ bool sbbs_t::editfiledesc(file_t* f)
 	if (f->desc != NULL && strcmp(fdesc, f->desc) == 0)
 		return true;
 	smb_new_hfield_str(f, SMB_FILEDESC, fdesc);
-	return updatefile(&cfg, f);
+	return updatefile(&cfg, f, NULL);
 }
 
 bool sbbs_t::editfileinfo(file_t* f)
@@ -365,5 +366,5 @@ bool sbbs_t::editfileinfo(file_t* f)
 			return false;
 		inputnstime32((time32_t*)&f->hdr.when_imported.time);
 	}
-	return updatefile(&cfg, f);
+	return updatefile(&cfg, f, NULL);
 }
diff --git a/src/sbbs3/filedat.c b/src/sbbs3/filedat.c
index 554e7bc716..b771bbe01c 100644
--- a/src/sbbs3/filedat.c
+++ b/src/sbbs3/filedat.c
@@ -373,18 +373,22 @@ void freefiles(file_t* filelist, size_t count)
 	free(filelist);
 }
 
-bool loadfile(scfg_t* cfg, int dirnum, const char* filename, file_t* file, enum file_detail detail)
+bool loadfile(scfg_t* cfg, int dirnum, const char* filename, file_t* file, enum file_detail detail, int* result)
 {
+	int   retval;
 	smb_t smb;
 
-	if (smb_open_dir(cfg, &smb, dirnum) != SMB_SUCCESS)
+	if (result == NULL)
+		result = &retval;
+
+	if (((*result) = smb_open_dir(cfg, &smb, dirnum)) != SMB_SUCCESS)
 		return false;
 
-	int result = smb_loadfile(&smb, filename, file, detail);
+	*result = smb_loadfile(&smb, filename, file, detail);
 	smb_close(&smb);
 	if (cfg->dir[dirnum]->misc & DIR_FREE)
 		file->cost = 0;
-	return result == SMB_SUCCESS;
+	return (*result) == SMB_SUCCESS;
 }
 
 char* batch_list_name(scfg_t* cfg, uint usernumber, enum XFER_TYPE type, char* fname, size_t size)
@@ -550,31 +554,39 @@ bool batch_file_load(scfg_t* cfg, str_list_t ini, const char* filename, file_t*
 	f->dir = batch_file_dir(cfg, ini, filename);
 	if (f->dir < 0)
 		return false;
-	return loadfile(cfg, f->dir, filename, f, file_detail_normal);
+	return loadfile(cfg, f->dir, filename, f, file_detail_normal, NULL);
 }
 
-bool updatefile(scfg_t* cfg, file_t* file)
+bool updatefile(scfg_t* cfg, file_t* file, int* result)
 {
+	int   retval;
 	smb_t smb;
 
-	if (smb_open_dir(cfg, &smb, file->dir) != SMB_SUCCESS)
+	if (result == NULL)
+		result = &retval;
+
+	if (((*result) = smb_open_dir(cfg, &smb, file->dir)) != SMB_SUCCESS)
 		return false;
 
-	int result = smb_updatemsg(&smb, file);
+	*result = smb_updatemsg(&smb, file);
 	smb_close(&smb);
-	return result == SMB_SUCCESS;
+	return (*result) == SMB_SUCCESS;
 }
 
-bool removefile(scfg_t* cfg, int dirnum, const char* filename)
+bool removefile(scfg_t* cfg, int dirnum, const char* filename, int* result)
 {
+	int retval;
 	smb_t smb;
 
-	if (smb_open_dir(cfg, &smb, dirnum) != SMB_SUCCESS)
+	if (result == NULL)
+		result = &retval;
+
+	if (((*result) = smb_open_dir(cfg, &smb, dirnum)) != SMB_SUCCESS)
 		return false;
 
-	int result = smb_removefile_by_name(&smb, filename);
+	*result = smb_removefile_by_name(&smb, filename);
 	smb_close(&smb);
-	return result == SMB_SUCCESS;
+	return (*result) == SMB_SUCCESS;
 }
 
 ulong getuserxfers(scfg_t* cfg, const char* from, uint to)
@@ -736,20 +748,24 @@ int file_sauce_hfields(file_t* f, struct sauce_charinfo* info)
 	return SMB_SUCCESS;
 }
 
-bool addfile(scfg_t* cfg, file_t* f, const char* extdesc, const char* metadata, client_t* client)
+bool addfile(scfg_t* cfg, file_t* f, const char* extdesc, const char* metadata, client_t* client, int* result)
 {
 	char  fpath[MAX_PATH + 1];
+	int   retval;
 	smb_t smb;
 
-	if (smb_open_dir(cfg, &smb, f->dir) != SMB_SUCCESS)
+	if (result == NULL)
+		result = &retval;
+
+	if (((*result) = smb_open_dir(cfg, &smb, f->dir)) != SMB_SUCCESS)
 		return false;
 
 	getfilepath(cfg, f, fpath);
 	if (f->from_ip == NULL)
 		file_client_hfields(f, client);
-	int result = smb_addfile(&smb, f, SMB_SELFPACK, extdesc, metadata, fpath);
+	*result = smb_addfile(&smb, f, SMB_SELFPACK, extdesc, metadata, fpath);
 	smb_close(&smb);
-	return result == SMB_SUCCESS;
+	return (*result) == SMB_SUCCESS;
 }
 
 /* 'size' does not include the NUL-terminator */
diff --git a/src/sbbs3/filedat.h b/src/sbbs3/filedat.h
index e129ec3574..28dba7e759 100644
--- a/src/sbbs3/filedat.h
+++ b/src/sbbs3/filedat.h
@@ -39,13 +39,13 @@ DLLEXPORT time_t		dir_newfiletime(scfg_t*, int dirnum);
 DLLEXPORT time_t		lastfiletime(smb_t*); // Reads the last index record
 
 DLLEXPORT bool			findfile(scfg_t* cfg, int dirnum, const char *filename, file_t*);
-DLLEXPORT bool			loadfile(scfg_t*, int dirnum, const char* filename, file_t*, enum file_detail);
+DLLEXPORT bool			loadfile(scfg_t*, int dirnum, const char* filename, file_t*, enum file_detail, int* result);
 DLLEXPORT file_t*		loadfiles(smb_t*, const char* filespec, time_t, enum file_detail, enum file_sort, size_t* count);
 DLLEXPORT void			sortfiles(file_t*, size_t count, enum file_sort);
 DLLEXPORT void			freefiles(file_t*, size_t count);
 DLLEXPORT str_list_t	loadfilenames(smb_t*, const char* filespec, time_t t, enum file_sort, size_t* count);
 DLLEXPORT void			sortfilenames(str_list_t, size_t count, enum file_sort);
-DLLEXPORT bool			updatefile(scfg_t*, file_t*);
+DLLEXPORT bool			updatefile(scfg_t*, file_t*, int* result);
 DLLEXPORT char*			getfilepath(scfg_t*, file_t*, char* path);
 DLLEXPORT char*			getfilevpath(scfg_t*, file_t*, char* path, size_t);
 DLLEXPORT off_t			getfilesize(scfg_t*, file_t*);
@@ -53,8 +53,8 @@ DLLEXPORT time_t		getfiletime(scfg_t*, file_t*);
 DLLEXPORT ulong			gettimetodl(scfg_t*, file_t*, uint rate_cps);
 DLLEXPORT ulong			getuserxfers(scfg_t*, const char* from, uint to);
 DLLEXPORT bool			hashfile(scfg_t*, file_t*);
-DLLEXPORT bool			addfile(scfg_t*, file_t*, const char* extdesc, const char* metadata, client_t*);
-DLLEXPORT bool			removefile(scfg_t*, int dirnum, const char* filename);
+DLLEXPORT bool			addfile(scfg_t*, file_t*, const char* extdesc, const char* metadata, client_t*, int* result);
+DLLEXPORT bool			removefile(scfg_t*, int dirnum, const char* filename, int* result);
 DLLEXPORT char*			format_filename(const char* fname, char* buf, size_t, bool pad);
 DLLEXPORT bool			safest_filename(const char* fname);
 DLLEXPORT bool			illegal_filename(const char* fname);
diff --git a/src/sbbs3/ftpsrvr.c b/src/sbbs3/ftpsrvr.c
index 4454d906b8..10ff9cae4f 100644
--- a/src/sbbs3/ftpsrvr.c
+++ b/src/sbbs3/ftpsrvr.c
@@ -778,7 +778,7 @@ static void send_thread(void* arg)
 
 		if (xfer.dir >= 0 && !xfer.tmpfile) {
 			memset(&f, 0, sizeof(f));
-			if (!loadfile(&scfg, xfer.dir, getfname(xfer.filename), &f, file_detail_normal)) {
+			if (!loadfile(&scfg, xfer.dir, getfname(xfer.filename), &f, file_detail_normal, NULL)) {
 				lprintf(LOG_ERR, "%04d <%s> DATA downloaded: %s (not found in filebase!)"
 				        , xfer.ctrl_sock
 				        , xfer.user->alias
@@ -786,7 +786,7 @@ static void send_thread(void* arg)
 			} else {
 				f.hdr.times_downloaded++;
 				f.hdr.last_downloaded = time32(NULL);
-				updatefile(&scfg, &f);
+				updatefile(&scfg, &f, NULL);
 
 				lprintf(LOG_INFO, "%04d <%s> DATA downloaded: %s (%u times total)"
 				        , xfer.ctrl_sock
@@ -1108,20 +1108,22 @@ static void receive_thread(void* arg)
 			if (f.desc == NULL)
 				smb_new_hfield_str(&f, SMB_FILEDESC, fdesc);
 			if (filedat) {
-				if (updatefile(&scfg, &f))
+				int result;
+				if (updatefile(&scfg, &f, &result))
 					lprintf(LOG_INFO, "%04d <%s> DATA updated file: %s"
 					        , xfer.ctrl_sock, xfer.user->alias, f.name);
 				else
-					lprintf(LOG_ERR, "%04d <%s> !DATA ERROR updating file (%s) in database"
-					        , xfer.ctrl_sock, xfer.user->alias, f.name);
+					lprintf(LOG_ERR, "%04d <%s> !DATA ERROR %d updating file (%s) in database"
+					        , xfer.ctrl_sock, xfer.user->alias, result, f.name);
 				/* need to update the index here */
 			} else {
-				if (addfile(&scfg, &f, extdesc, /* metatdata: */ NULL, xfer.client))
+				int result;
+				if (addfile(&scfg, &f, extdesc, /* metatdata: */ NULL, xfer.client, &result))
 					lprintf(LOG_INFO, "%04d <%s> DATA uploaded file: %s"
 					        , xfer.ctrl_sock, xfer.user->alias, f.name);
 				else
-					lprintf(LOG_ERR, "%04d <%s> !DATA ERROR adding file (%s) to database"
-					        , xfer.ctrl_sock, xfer.user->alias, f.name);
+					lprintf(LOG_ERR, "%04d <%s> !DATA ERROR %d adding file (%s) to database"
+					        , xfer.ctrl_sock, xfer.user->alias, result, f.name);
 			}
 
 			if (scfg.dir[f.dir]->upload_sem[0])
@@ -4560,7 +4562,7 @@ static void ctrl_thread(void* arg)
 				    && !download_is_free(&scfg, dir, &user, &client)) {
 					file_t f;
 					if (filedat)
-						loadfile(&scfg, dir, p, &f, file_detail_normal);
+						loadfile(&scfg, dir, p, &f, file_detail_normal, NULL);
 					else
 						f.cost = (uint32_t)flength(fname);
 					if (f.cost > user_available_credits(&user)) {
@@ -4615,7 +4617,7 @@ static void ctrl_thread(void* arg)
 				} else {
 					lprintf(LOG_NOTICE, "%04d <%s> deleted %s", sock, user.alias, fname);
 					if (filedat)
-						removefile(&scfg, dir, getfname(fname));
+						removefile(&scfg, dir, getfname(fname), NULL);
 					sockprintf(sock, sess, "250 %s deleted.", fname);
 				}
 			} else if (success) {
@@ -4779,7 +4781,7 @@ static void ctrl_thread(void* arg)
 				}
 				if (append || filepos) { /* RESUME */
 					file_t f;
-					if (!loadfile(&scfg, dir, p, &f, file_detail_normal)) {
+					if (!loadfile(&scfg, dir, p, &f, file_detail_normal, NULL)) {
 						if (filepos) {
 							lprintf(LOG_WARNING, "%04d <%s> file (%s) not in database for %.4s command"
 							        , sock, user.alias, fname, cmd);
diff --git a/src/sbbs3/listfile.cpp b/src/sbbs3/listfile.cpp
index d770b2b7c7..3be3c3b341 100644
--- a/src/sbbs3/listfile.cpp
+++ b/src/sbbs3/listfile.cpp
@@ -923,7 +923,7 @@ int sbbs_t::listfileinfo(const int dirnum, const char *filespec, const int mode)
 							         , f->name, cfg.lib[cfg.dir[i]->lib]->sname, cfg.dir[i]->sname);
 							if (yesno(str)) {
 								f->dir = i;
-								addfile(&cfg, f, f->extdesc, f->auxdata, /* client: */ NULL);
+								addfile(&cfg, f, f->extdesc, f->auxdata, /* client: */ NULL, /* result: */NULL);
 							}
 						}
 					}
diff --git a/src/sbbs3/qwk.cpp b/src/sbbs3/qwk.cpp
index 77ae997b4a..e9941739be 100644
--- a/src/sbbs3/qwk.cpp
+++ b/src/sbbs3/qwk.cpp
@@ -885,7 +885,7 @@ void sbbs_t::qwkcfgline(char *buf, int subnum)
 		SKIP_WHITESPACE(fname);
 		for (x = y = 0; x < usrlibs; x++) {
 			for (y = 0; y < usrdirs[x]; y++)
-				if (loadfile(&cfg, usrdir[x][y], fname, &f, file_detail_normal))
+				if (loadfile(&cfg, usrdir[x][y], fname, &f, file_detail_normal, /* result: */NULL))
 					break;
 			if (y < usrdirs[x])
 				break;
diff --git a/src/sbbs3/upload.cpp b/src/sbbs3/upload.cpp
index 9e0b1d36b0..0a30133a10 100644
--- a/src/sbbs3/upload.cpp
+++ b/src/sbbs3/upload.cpp
@@ -172,7 +172,7 @@ bool sbbs_t::uploadfile(file_t* f)
 	smb_hfield_bin(f, SMB_COST, length);
 	smb_hfield_str(f, SENDER, useron.alias);
 	bprintf(text[FileNBytesReceived], f->name, u64toac(length, tmp));
-	if (!addfile(&cfg, f, ext, /* metadata: */ NULL, &client))
+	if (!addfile(&cfg, f, ext, /* metadata: */ NULL, &client, NULL))
 		return false;
 
 	snprintf(str, sizeof(str), "uploaded %s (%" PRId64 " bytes) to %s %s"
diff --git a/src/sbbs3/userdat.c b/src/sbbs3/userdat.c
index db4fad31e7..c52e5083c7 100644
--- a/src/sbbs3/userdat.c
+++ b/src/sbbs3/userdat.c
@@ -3022,7 +3022,7 @@ bool user_downloaded_file(scfg_t* cfg, user_t* user, client_t* client,
 	bool   removed = false;
 
 	filename = getfname(filename);
-	if (!loadfile(cfg, dirnum, filename, &f, file_detail_normal))
+	if (!loadfile(cfg, dirnum, filename, &f, file_detail_normal, NULL))
 		return false;
 
 	if (!bytes)
@@ -3042,14 +3042,14 @@ bool user_downloaded_file(scfg_t* cfg, user_t* user, client_t* client,
 		if (strListCount(dest_user_list) < 1) {
 			char path[MAX_PATH + 1];
 			if (remove(getfilepath(cfg, &f, path)) == 0)
-				removed = removefile(cfg, dirnum, f.name);
+				removed = removefile(cfg, dirnum, f.name, NULL);
 		}
 		strListFree(&dest_user_list);
 	}
 
 	f.hdr.times_downloaded++;
 	f.hdr.last_downloaded = time32(NULL);
-	if (!removed && !updatefile(cfg, &f)) {
+	if (!removed && !updatefile(cfg, &f, NULL)) {
 		smb_freefilemem(&f);
 		return false;
 	}
diff --git a/src/sbbs3/websrvr.c b/src/sbbs3/websrvr.c
index e6e06f8027..5c51eecb19 100644
--- a/src/sbbs3/websrvr.c
+++ b/src/sbbs3/websrvr.c
@@ -3283,7 +3283,7 @@ static enum parsed_vpath resolve_vpath(http_session_t* session, char* vpath)
 	safe_snprintf(path, sizeof(path), "%s%s", scfg.dir[session->file.dir]->path, filename);
 	if (!fexistcase(path))
 		return PARSED_VPATH_NONE;
-	if (!loadfile(&scfg, session->file.dir, filename, &session->file, file_detail_index))
+	if (!loadfile(&scfg, session->file.dir, filename, &session->file, file_detail_index, NULL))
 		return PARSED_VPATH_NONE;
 	strlcpy(vpath, path, MAX_PATH);
 	return PARSED_VPATH_FULL;
-- 
GitLab