Add/user sbbs_t::getuseron() for better user.tab error logging/handling

Looking into potential causes of issue #843, I found several instances where
we call getuserdat() without checking the return value and restoring the
useron.number to the current user number upon error: getuserdat() zeroes out
the user struct/number upon error, a bad API choice made 23 years ago.

Replace those instances with calls to sbbs_t::getuseron() which logs any
open/lock/read failures of the user.tab and does not modify/zero-out the
sbbs_t::useron struct upon error.

src/sbbs3/chat.cpp

@@ -1118,8 +1118,7 @@ void sbbs_t::privchat(bool forced, int node_num)
 					if(thisnode.misc&NODE_INTR)
 						break;
 					if(thisnode.misc&NODE_UDAT && !(useron.rest&FLAG('G'))) {
-						getuserdat(&cfg,&useron);
-						if(getnodedat(cfg.node_num,&thisnode, true)) {
+						if(getuseron(WHERE) && getnodedat(cfg.node_num,&thisnode, true)) {
 							thisnode.misc&=~NODE_UDAT;
 							putnodedat(cfg.node_num,&thisnode);
 						}

src/sbbs3/data.cpp

@@ -297,3 +297,25 @@ uint sbbs_t::gettimeleft(bool handle_out_of_time)
 	}
 	return timeleft;
 }
+
+/****************************************************************************/
+/* Read the user.tab record for the specified usernumber					*/
+/* (or current useron.number) into 'useron'.								*/
+/* Logs error and does *not* change/zero-out 'useron' upon failure			*/
+/****************************************************************************/
+bool sbbs_t::getuseron(int line, const char* function, const char *source, uint usernum)
+{
+	user_t user{};
+	if(usernum == 0)
+		usernum = useron.number;
+	user.number = usernum;
+	int retval = getuserdat(&cfg, &user);
+	if(retval == USER_SUCCESS) {
+		useron = user;
+		return true;
+	}
+	char str[128];
+	snprintf(str, sizeof str, "getuserdat returned %d", retval);
+	errormsg(line, function, source, ERR_READ, USER_DATA_FILENAME, usernum, str);
+	return false;
+}

src/sbbs3/execfunc.cpp

@@ -54,7 +54,7 @@ int sbbs_t::exec_function(csi_t *csi)
 		case CS_USER_DEFAULTS:
 			maindflts(&useron);
 			if(!(useron.rest&FLAG('G')))    /* not guest */
-				getuserdat(&cfg,&useron);
+				getuseron(WHERE);
 			return(0);
 		case CS_TEXT_FILE_SECTION:
 			text_sec();

src/sbbs3/getnode.cpp

@@ -151,8 +151,7 @@ void sbbs_t::nodesync(bool clearline)
 			} 
 		}
 		if(thisnode.misc&NODE_UDAT && !(useron.rest&FLAG('G'))) {   /* not guest */
-			getuserdat(&cfg, &useron);
-			if(getnodedat(cfg.node_num,&thisnode, true)) {
+			if(getuseron(WHERE) && getnodedat(cfg.node_num,&thisnode, true)) {
 				thisnode.misc&=~NODE_UDAT;
 				putnodedat(cfg.node_num,&thisnode); 
 			}

src/sbbs3/js_bbs.cpp

@@ -2517,7 +2517,7 @@ js_user_config(JSContext *cx, uintN argc, jsval *arglist)
 	rc=JS_SUSPENDREQUEST(cx);
 	sbbs->maindflts(&sbbs->useron);
 	if(!(sbbs->useron.rest&FLAG('G')))    /* not guest */
-		getuserdat(&sbbs->cfg,&sbbs->useron);
+		sbbs->getuseron(WHERE);
 	JS_RESUMEREQUEST(cx, rc);
 
 	return(JS_TRUE);
@@ -2535,7 +2535,7 @@ js_user_sync(JSContext *cx, uintN argc, jsval *arglist)
 	JS_SET_RVAL(cx, arglist, JSVAL_VOID);
 
 	rc=JS_SUSPENDREQUEST(cx);
-	getuserdat(&sbbs->cfg,&sbbs->useron);
+	sbbs->getuseron(WHERE);
 	JS_RESUMEREQUEST(cx, rc);
 
 	return(JS_TRUE);

src/sbbs3/main.cpp

@@ -2965,9 +2965,7 @@ void event_thread(void* arg)
 					continue;
 				sbbs->useron.number = 0;
 				sbbs->lprintf(LOG_DEBUG, "Inbound QWK Reply Packet detected: %s", fname);
-				sbbs->useron.number = atoi(fname+offset);
-				getuserdat(&sbbs->cfg,&sbbs->useron);
-				if(sbbs->useron.number != 0 && !(sbbs->useron.misc&(DELETED|INACTIVE))) {
+				if(sbbs->getuseron(WHERE, atoi(fname+offset)) && !(sbbs->useron.misc&(DELETED|INACTIVE))) {
 					fmutex_t lockfile;
 					SAFEPRINTF(lockfile.name,"%s.lock",fname);
 					if(!fmutex_open(&lockfile, startup->host_name, TIMEOUT_MUTEX_FILE)) {
@@ -3028,16 +3026,12 @@ void event_thread(void* arg)
 					continue;
 				sbbs->useron.number = 0;
 				sbbs->lprintf(LOG_INFO, "QWK pack semaphore signaled: %s", fname);
-				int usernum = atoi(fname+offset);
-				sbbs->useron.number = usernum;
-				int retval = getuserdat(&sbbs->cfg,&sbbs->useron);
-				if(retval != 0) {
-					sbbs->lprintf(LOG_WARNING, "ERROR %d reading user data for user #%d", retval, usernum);
+				if(!sbbs->getuseron(WHERE, atoi(fname+offset))) {
 					sbbs->fremove(WHERE, fname, /* log-all-errors: */true);
 					continue;
 				}
 				fmutex_t lockfile;
-				SAFEPRINTF2(lockfile.name,"%spack%04u.lock",sbbs->cfg.data_dir,usernum);
+				SAFEPRINTF2(lockfile.name,"%spack%04u.lock",sbbs->cfg.data_dir, sbbs->useron.number);
 				if(!fmutex_open(&lockfile, startup->host_name, TIMEOUT_MUTEX_FILE)) {
 					if(difftime(time(NULL), lockfile.time) > 60)
 						sbbs->lprintf(LOG_INFO,"%s exists (pack in progress?) since %s", lockfile.name, time_as_hhmm(&sbbs->cfg, lockfile.time, str));
@@ -4437,8 +4431,7 @@ void node_thread(void* arg)
 				SLEEP(1000);
 				sbbs->getnodedat(sbbs->cfg.node_num, &sbbs->thisnode, /* lock: */false);
 				if(sbbs->thisnode.misc & NODE_UDAT && !(sbbs->useron.rest & FLAG('G'))) {   /* not guest */
-					getuserdat(&sbbs->cfg, &sbbs->useron);
-					if(sbbs->getnodedat(sbbs->cfg.node_num, &sbbs->thisnode, /* lock: */true)) {
+					if(sbbs->getuseron(WHERE) && sbbs->getnodedat(sbbs->cfg.node_num, &sbbs->thisnode, /* lock: */true)) {
 						sbbs->thisnode.misc &= ~NODE_UDAT;
 						sbbs->putnodedat(sbbs->cfg.node_num, &sbbs->thisnode); 
 					}

src/sbbs3/newuser.cpp

@@ -471,7 +471,7 @@ bool sbbs_t::newuser()
 	if(cfg.newuser_mod[0])
 		exec_bin(cfg.newuser_mod,&main_csi);
 	user_event(EVENT_NEWUSER);
-	getuserdat(&cfg,&useron);	// In case event(s) modified user data
+	getuseron(WHERE);	// In case event(s) modified user data
 	logline("N+","Successful new user logon");
 
 	return(true);

src/sbbs3/sbbs.h

@@ -795,6 +795,7 @@ public:
 	int		getusrgrp(int subnum);
 	int		getusrdir(int dirnum);
 	int		getusrlib(int dirnum);
+	bool	getuseron(int line, const char* function, const char *source, uint usernum = 0);
 
 	bool	putuserstr(int usernumber, enum user_field, const char *str);
 	bool	putuserdatetime(int usernumber, enum user_field, time_t t);