Refactor putuserrec()

This started with a Coverity issue (CID 33230) which got me looking at this function and wondering: why is str2 being NUL-terminated here? Why is the length of str2 to be calculated on successive lines? What is with this (long)((long)((long)((long)) typecast?

This was some of the oldest code in Synchronet (along with a lot of the other functions in this file). I tried to keep as much intact as possible while still improving the logic and readability.

src/sbbs3/userdat.c

@@ -2217,66 +2217,73 @@ int getuserrec(scfg_t* cfg, int usernumber,int start, int length, char *str)
 }
 
 /****************************************************************************/
-/* Places into user.dat at the offset for usernumber+start for length bytes */
-/* Called from various locations											*/
+/* Write a string (str) into user.dat at the offset for usernumber + start	*/
+/* 'length' may be auto-determined (from 'start') by passing 0 for length	*/
+/* If 'str' is longer than 'length', only 'length' characters are written	*/
 /****************************************************************************/
-int putuserrec(scfg_t* cfg, int usernumber,int start, uint length, const char *str)
+int putuserrec(scfg_t* cfg, int usernumber, int start, int length, const char *str)
 {
-	char	str2[256];
+	const char*	p;
+	char	buf[256];
+	char	path[MAX_PATH + 1];
 	int		file;
-	uint	c,i;
+	uint	i;
 
-	if(!VALID_CFG(cfg) || usernumber<1 || str==NULL)
-		return(-1);
+	if(!VALID_CFG(cfg) || usernumber < 1 || str == NULL)
+		return -1;
 
-	if(length > sizeof(str2) - 1)
-		return -10;
+	if(length == 0) {	/* auto-length */
+		length = user_rec_len(start);
+		if(length < 1)
+			return -2;
+	}
+	size_t slen = strlen(str);
+	if(slen >= (size_t)length)
+		p = str;
+	else {
+		if(length > sizeof(buf))
+			return -10;
+		memset(buf, ETX, length);
+		memcpy(buf, str, slen);
+		p = buf;
+	}
 
-	SAFEPRINTF(str2,"%suser/user.dat",cfg->data_dir);
-	if((file=nopen(str2,O_RDWR|O_DENYNONE))==-1)
-		return(errno);
+	SAFEPRINTF(path, "%suser/user.dat", cfg->data_dir);
+	if((file = nopen(path, O_RDWR|O_DENYNONE))==-1)
+		return errno;
 
-	if(filelength(file)<((long)usernumber-1)*U_LEN) {
+	off_t offset = (usernumber - 1) * U_LEN;
+	if(filelength(file) < offset) {
 		close(file);
-		return(-4);
-	}
-
-	if(length==0) {	/* auto-length */
-		length=user_rec_len(start);
-		if((long)length < 0) {
-			close(file);
-			return -2;
-		}
+		return -4;
 	}
 
-	SAFECOPY(str2,str);
-	if(strlen(str2)<length) {
-		for(c=strlen(str2);c<length;c++)
-			str2[c]=ETX;
-		str2[c]=0;
+	offset += start;
+	if(lseek(file, offset, SEEK_SET) != offset) {
+		close(file);
+		return -5;
 	}
-	lseek(file,(long)((long)((long)((long)usernumber-1)*U_LEN)+start),SEEK_SET);
 
 	i=0;
-	while(i<LOOP_NODEDAB
-		&& lock(file,(long)((long)(usernumber-1)*U_LEN)+start,length)==-1) {
+	while(i < LOOP_NODEDAB
+		&& lock(file, offset, length) == -1) {
 		if(i)
 			mswait(100);
 		i++;
 	}
 
-	if(i>=LOOP_NODEDAB) {
+	if(i >= LOOP_NODEDAB) {
 		close(file);
-		return(-3);
+		return -3;
 	}
 
-	int wr = write(file, str2, length);
-	unlock(file,(long)((long)(usernumber-1)*U_LEN)+start,length);
+	int wr = write(file, p, length);
+	unlock(file, offset, length);
 	close(file);
 	if(wr != length)
-		return -4;
-	dirtyuserdat(cfg,usernumber);
-	return(0);
+		return -6;
+	dirtyuserdat(cfg, usernumber);
+	return 0;
 }
 
 /****************************************************************************/

src/sbbs3/userdat.h

@@ -81,7 +81,7 @@ DLLEXPORT uint	userdatdupe(scfg_t*, uint usernumber, uint offset, uint datlen, c
 DLLEXPORT BOOL	chk_ar(scfg_t*, uchar* str, user_t*, client_t*); /* checks access requirements */
 
 DLLEXPORT int	getuserrec(scfg_t*, int usernumber, int start, int length, char *str);
-DLLEXPORT int	putuserrec(scfg_t*, int usernumber, int start, uint length, const char *str);
+DLLEXPORT int	putuserrec(scfg_t*, int usernumber, int start, int length, const char *str);
 DLLEXPORT ulong	adjustuserrec(scfg_t*, int usernumber, int start, int length, long adj);
 DLLEXPORT BOOL	logoutuserdat(scfg_t*, user_t*, time_t now, time_t logontime);
 DLLEXPORT void	resetdailyuserdat(scfg_t*, user_t*, BOOL write);