Declares correct size for scratch version string storage

Replaces LispStringToCString macro with static inline procedure
Adds error returns for case where new file version would exceed MAXVERSION
2026-03-15 14:27:19 +00:00 · 2025-07-25 14:27:22 -07:00 · 2025-07-25 14:27:22 -07:00 · 2025-07-25 14:27:22 -07:00 · 2025-07-25 14:27:22 -07:00 · 2025-07-25 14:27:22 -07:00
3 changed files with 210 additions and 184 deletions
--- a/inc/lispver2.h
+++ b/inc/lispver2.h
@@ -9,7 +9,7 @@
 	char	*lv_cp;						\
 	char	*lv_vp;						\
 	unsigned lv_ver;					\
-	char		lv_ver_buf[VERSIONLEN];				\
+	char		lv_ver_buf[VERSIONLEN + 3];			\
 									\
 	lv_cp = pathname;						\
 	lv_vp = NULL;							\
--- a/inc/locfile.h
+++ b/inc/locfile.h
@@ -8,10 +8,14 @@
 /*	Manufactured in the United States of America.			*/
 /*									*/
 /************************************************************************/
+#include <ctype.h>     /* for isdigit */
 #include <errno.h>
 #include <limits.h>   /* for NAME_MAX */
+#include <string.h>   /* for strlen */
+#include <sys/param.h> /* for MAXPATHLEN */
 #include <dirent.h>   /* for MAXNAMLEN */
 #include "lispemul.h" /* for DLword */
+#include "commondefs.h" /* for error */

 #define	FDEV_PAGE_SIZE		512	/* 1 page == 512 byte */

@@ -63,6 +67,9 @@
        else if(((ptr) & SEGMASK)== S_NEGATIVE) {(place) = (int)((ptr)| 0xffff0000);}\
        else {return(NIL);}} while (0)

+#ifndef min
+#define min(a, b) (((a) <= (b))?(a):(b))
+#endif /* min */

 /************************************************************************/
 /*									*/
@@ -79,65 +86,65 @@
 /*									*/
 /************************************************************************/
 #ifndef BYTESWAP
-#define	LispStringToCString(Lisp, C, MaxLen)				\
-  do {									\
-    OneDArray	*lf_arrayp;						\
-    char	*lf_base, *lf_dp;						\
-    short	*lf_sbase;							\
-    size_t		 lf_length;						\
-    lf_arrayp = (OneDArray *)NativeAligned4FromLAddr(Lisp);		\
-    lf_length = min(MaxLen, lf_arrayp->fillpointer);			\
-    switch(lf_arrayp->typenumber)						\
-      {									\
-	case THIN_CHAR_TYPENUMBER:					\
-		lf_base = ((char *)(NativeAligned2FromLAddr(lf_arrayp->base)))  	\
-		       + ((int)(lf_arrayp->offset));			\
-		strncpy(C, lf_base, lf_length);				\
-		(C)[lf_length] = '\0';					\
-		break;							\
-									\
-	case FAT_CHAR_TYPENUMBER:					\
-		lf_sbase = ((short *)(NativeAligned2FromLAddr(lf_arrayp->base)))	\
-		       + ((int)(lf_arrayp->offset));			\
-                lf_dp = C;						\
-		for(size_t lf_i=0;lf_i<(lf_length);lf_i++)		\
-		  *lf_dp++ = (char)(*lf_sbase++);			\
-		*lf_dp = '\0';						\
-		break;							\
-	default:							\
-		error("LispStringToCString: Not a character array.\n");	\
-      }									\
- } while (0)
+static inline void LispStringToCString(LispPTR Lisp, char *C, size_t MaxLen)
+{
+    OneDArray	*lf_arrayp;
+    char	*lf_base, *lf_dp;
+    short	*lf_sbase;
+    size_t		 lf_length;
+    lf_arrayp = (OneDArray *)NativeAligned4FromLAddr(Lisp);
+    lf_length = min(MaxLen, lf_arrayp->fillpointer);
+    switch(lf_arrayp->typenumber)
+      {
+	case THIN_CHAR_TYPENUMBER:
+		lf_base = ((char *)(NativeAligned2FromLAddr(lf_arrayp->base)))
+		       + ((int)(lf_arrayp->offset));
+		strncpy(C, lf_base, lf_length);
+		(C)[lf_length] = '\0';
+		break;
+
+	case FAT_CHAR_TYPENUMBER:
+		lf_sbase = ((short *)(NativeAligned2FromLAddr(lf_arrayp->base)))
+		       + ((int)(lf_arrayp->offset));
+                lf_dp = C;
+		for(size_t lf_i=0;lf_i<(lf_length);lf_i++)
+		  *lf_dp++ = (char)(*lf_sbase++);
+		*lf_dp = '\0';
+		break;
+	default:
+		error("LispStringToCString: Not a character array.\n");
+      }
+}
 #else  /* BYTESWAP == T CHANGED-BY-TAKE */
-#define	LispStringToCString(Lisp, C, MaxLen)				\
-  do {									\
-    OneDArray	*lf_arrayp;						\
-    char	*lf_base, *lf_dp;						\
-    short	*lf_sbase;							\
-    size_t 	lf_length;						\
-    lf_arrayp = (OneDArray *)(NativeAligned4FromLAddr(Lisp));			\
-    lf_length = min(MaxLen, lf_arrayp->fillpointer);			\
-    switch(lf_arrayp->typenumber)						\
-      {									\
-	case THIN_CHAR_TYPENUMBER:					\
-		lf_base = ((char *)(NativeAligned2FromLAddr(lf_arrayp->base)))  	\
-		       + ((int)(lf_arrayp->offset));			\
-		StrNCpyFromLispToC(C , lf_base , lf_length );		\
-		(C)[lf_length] = '\0';					\
-		break;							\
-									\
-	case FAT_CHAR_TYPENUMBER:					\
-		lf_sbase = ((short *)(NativeAligned2FromLAddr(lf_arrayp->base)))	\
-		       + ((int)(lf_arrayp->offset));			\
-                lf_dp = C;						\
-		for(size_t lf_ii=0;lf_ii<(lf_length);lf_ii++,lf_sbase++)  \
-                    *lf_dp++ = (char)(GETWORD(lf_sbase));               \
-		*lf_dp = '\0';						\
-		break;							\
-	default:							\
-		error("LispStringToCString: Not a character array.\n");	\
-      }									\
-  } while (0)
+static inline void LispStringToCString(LispPTR Lisp, char *C, size_t MaxLen)
+{
+    OneDArray	*lf_arrayp;
+    char	*lf_base, *lf_dp;
+    short	*lf_sbase;
+    size_t 	lf_length;
+    lf_arrayp = (OneDArray *)(NativeAligned4FromLAddr(Lisp));
+    lf_length = min(MaxLen, lf_arrayp->fillpointer);
+    switch(lf_arrayp->typenumber)
+      {
+	case THIN_CHAR_TYPENUMBER:
+		lf_base = ((char *)(NativeAligned2FromLAddr(lf_arrayp->base)))
+		       + ((int)(lf_arrayp->offset));
+		StrNCpyFromLispToC(C , lf_base , lf_length );
+		(C)[lf_length] = '\0';
+		break;
+
+	case FAT_CHAR_TYPENUMBER:
+		lf_sbase = ((short *)(NativeAligned2FromLAddr(lf_arrayp->base)))
+		       + ((int)(lf_arrayp->offset));
+                lf_dp = C;
+		for(size_t lf_ii=0;lf_ii<(lf_length);lf_ii++,lf_sbase++)
+                    *lf_dp++ = (char)(GETWORD(lf_sbase));
+		*lf_dp = '\0';
+		break;
+	default:
+		error("LispStringToCString: Not a character array.\n");
+      }
+}

 #endif /* BYTESWAP */

@@ -189,10 +196,6 @@ do  {				\
 	(cstringp) = (char *)(NativeAligned2FromLAddr(((OneDArray *)lf_naddress)->base)); \
 } while (0)

-#ifndef min
-#define min(a, b) (((a) <= (b))?(a):(b))
-#endif /* min */
-
 #define	LispNumToCInt(Lisp)					\
       ( (((Lisp) & SEGMASK) == S_POSITIVE) ? ((Lisp) & 0xFFFF) : \
          (((Lisp) & SEGMASK) == S_NEGATIVE) ? ((Lisp) | 0xFFFF0000) : \
@@ -293,7 +296,7 @@ do  {				\
 	goto truetag;	/* NOLINT(bugprone-macro-parentheses) */	\
  } while (0)

-/*		
+/*
 * Name:	LispVersionToUnixVersion
 *
 * Argument:	char	*pathname
@@ -328,17 +331,13 @@ do  {				\
 #endif /* DOS */


-/*		
+/*
 * Name:	UnixVersionToLispVersion
 *
- * Argument:	char	*pathname
- *				UNIX syntax pathname.
- *		int	vlessp
- *				If 0, versionless file is converted to version 1.
+ * Argument:	char *pathname  UNIX syntax pathname.
+ *		int vlessp      If 0, versionless file is converted to version 1.
 *				Otherwise, remains as versionless.
 *
- * Value:	If succeed, returns 1, otherwise 0.
- *
 * Side Effect:	The version part of pathname is destructively modified.
 *
 * Description:
@@ -350,67 +349,25 @@ do  {				\
 * version field will append a semicolon and it might make the routine be
 * confused.
 * The file which has not a valid version field, that is ".~##~" form, is
- * dealt with as version 1. 
+ * dealt with as version 1.
 */

-#define UnixVersionToLispVersion(pathname, vlessp) do {				\
-										\
-	char	*start;							\
-	char	*end;							\
-	char	*lf_cp;							\
-	int	ver_no;								\
-	size_t	len;								\
-	char		ver_buf[VERSIONLEN];					\
-										\
-	if ((start = strchr(pathname, '~')) != NULL) {				\
-		/* First of all, find the version field in pathname. */		\
-		end = start;							\
-		lf_cp = start + 1;							\
-		while (*lf_cp) {							\
-			if (*lf_cp == '~') {					\
-				start = end;					\
-				end = lf_cp;					\
-				lf_cp++;						\
-			} else {						\
-				lf_cp++;						\
-			}							\
-		}								\
-										\
-		if (start != end && *(start - 1) == '.' && end == (lf_cp - 1)) {	\
-			/*							\
-			 * pathname ends in the form ".~###~".  But we		\
-			 * check ### is a valid number or not.			\
-			 */							\
-			len = (end - start) - 1;			\
-			strncpy(ver_buf, start + 1, len);			\
-			ver_buf[len] = '\0';					\
-			NumericStringP(ver_buf, YES, NO);			\
-		      YES:							\
-			*(start - 1) = ';';					\
-			*start = '\0';						\
-			*end = '\0';						\
-			/* call strtoul() to eliminate leading 0s. */		\
-			ver_no = strtoul(start + 1, (char **)NULL, 10);		\
-			sprintf(ver_buf, "%u", ver_no);				\
-			strcat(pathname, ver_buf);				\
-			goto CONT;						\
-										\
-		      NO:							\
-			/* Dealt with as version 1 unless vlessp */		\
-			if (!(vlessp)) strcat(pathname, ";1");                  \
-		      CONT:							\
-			lf_cp--;	/* Just for label */			\
-		} else {							\
-			/* Dealt with as version 1 unless vlessp. */		\
-                        if (!(vlessp)) strcat(pathname, ";1");                  \
-		}								\
-	} else {								\
-		/* Dealt with as version 1 unless vlessp. */			\
-                if (!(vlessp)) strcat(pathname, ";1");                          \
-	}									\
-  } while (0)
+#define UnixVersionToLispVersion(pathname, vlessp)                      \
+  do {                                                                  \
+    char *n_end;                                                        \
+    char *v_start;                                                      \
+    int	v_len;                                                          \
+                                                                        \
+    if (!parse_file_version(pathname, 1, &n_end, &v_start, &v_len)) {   \
+      if (!vlessp) strcat(pathname, ";1");                              \
+    } else {                                                            \
+      *n_end++ = ';';                                                   \
+      while (v_len-- > 0) *n_end++ = *v_start++;                        \
+      *n_end = '\0';                                                    \
+    }                                                                   \
+  }  while (0)

-/*		
+/*
 * Name:	ConcDirAndName
 *
 * Argument:	char	*dir	The name of the directory.
@@ -428,45 +385,45 @@ do  {				\
 *
 */

-#define ConcDirAndName(dir, name, fname) do {					\
-									\
-	char	*lf_cp1, *lf_cp2;					\
-									\
-	lf_cp1 = dir;							\
-	lf_cp2 = dir;							\
-									\
-	while (*lf_cp2 != '\0') {						\
-		switch (*lf_cp2) {						\
-									\
-		      case '/':						\
-			lf_cp1 = lf_cp2;					\
-			lf_cp2++;						\
-			break;						\
-									\
-		      default:						\
-			lf_cp2++;						\
-			break;						\
-		}							\
-	}								\
-	if (lf_cp1 == (lf_cp2 - 1)) {						\
-                if (lf_cp1 == (dir)) {					\
-			/* dir is a root directory. */			\
-			strcpy(fname, "/");				\
-			strcat(fname, name);				\
-		} else {						\
-			/* The trail directory is included. */		\
-			strcpy(fname, dir);				\
-			strcat(fname, name);				\
-		}							\
-	} else {							\
-		/* The trail directory is not included */		\
-		strcpy(fname, dir);					\
-		strcat(fname, "/");					\
-		strcat(fname, name);					\
-	}								\
-  } while (0)
+static inline void ConcDirAndName(char *dir, char *name, char *fname)
+{
+  char	*lf_cp1, *lf_cp2;

-/*		
+  lf_cp1 = dir;
+  lf_cp2 = dir;
+
+  while (*lf_cp2 != '\0') {
+    switch (*lf_cp2) {
+
+    case '/':
+      lf_cp1 = lf_cp2;
+      lf_cp2++;
+      break;
+
+    default:
+      lf_cp2++;
+      break;
+    }
+  }
+  if (lf_cp1 == (lf_cp2 - 1)) {
+    if (lf_cp1 == (dir)) {
+      /* dir is a root directory. */
+      strcpy(fname, "/");
+      strcat(fname, name);
+    } else {
+      /* The trail directory is included. */
+      strcpy(fname, dir);
+      strcat(fname, name);
+    }
+  } else {
+    /* The trail directory is not included */
+    strcpy(fname, dir);
+    strcat(fname, "/");
+    strcat(fname, name);
+  }
+}
+
+/*
 * Name:	ConcNameAndVersion
 *
 * Argument:	char	*name	The root file name.
@@ -481,20 +438,24 @@ do  {				\
 *
 * Concatenate the root file name and its version in UNIX format.
 *
+ * XXX: this code is unsafe and could result in memory smashes if the
+ * sizes of the arguments are not correctly specified
+ *
 */

-#define ConcNameAndVersion(name, ver, rname) do {				\
-	if (*(ver) != '\0') {						\
-		strcpy(rname, name);					\
-		strcat(rname, ".~");					\
-		strcat(rname, ver);					\
-		strcat(rname, "~");					\
-	} else {							\
-		strcpy(rname, name);					\
-	}								\
-} while (0)
+static inline void ConcNameAndVersion(char *name, char *ver, char *rname)
+{
+	if (*ver != '\0') {
+		strcpy(rname, name);
+		strcat(rname, ".~");
+		strcat(rname, ver);
+		strcat(rname, "~");
+	} else {
+		strcpy(rname, name);
+	}
+}

-#define	 VERSIONLEN		16
+#define	 VERSIONLEN		10

 #define	MAXVERSION		999999999

@@ -542,7 +503,7 @@ do  {				\
 			*(lf_cp-1) = '\0';					\
 		}							\
  } while (0)
-		
+
 #define	ChangeToVersionless(pathname) do {			\
 		char	*lf_cp;			\
 		if( (lf_cp=strrchr(pathname, ';')) != 0)	\
@@ -576,9 +537,9 @@ do  {				\
 		TIMEOUT(lf_rval=rename(x, y));		\
 		if(lf_rval == -1){				\
 			switch(errno){			\
-			case 2:				\
+			case ENOENT:				\
 				return(1);		\
-			case 18:			\
+			case EXDEV:			\
 				*Lisp_errno = errno;	\
 				return(0);		\
 			default:			\
@@ -601,20 +562,78 @@ do  {				\
 /*
 * For file name length check
 */
-#define FNAMETOOLONG	200

-#define FileNameTooLong(val) do {				\
-	*Lisp_errno = FNAMETOOLONG;			\
+#define FileNameTooLong(val) do {			\
+	*Lisp_errno = ENAMETOOLONG;                     \
 	return((val));					\
  } while (0)

+static inline int parse_file_version(char *name, int digitsonly, char **n_end,
+                                      char **v_start, int *v_length)
+{
+  char *sp, *ep;
+  size_t name_len;

+  name_len = strlen(name);
+  ep = &name[name_len - 1];

+  /* handle special case of Alto/IFS names with !nnn version.
+     To be considered for this case the name MUST end with ![0-9]+, however
+     version 0 is not valid.
+   */
+  sp = strrchr(name, '!');
+  if (sp != NULL) {
+    sp++;         /* "!nnn" => "nnn" or "!" => ""  */
+    if (*sp != '\0' && sp[strspn(sp, "0123456789")] == '\0') {
+      /* it was all digits after the '!', so go with it */
+      *n_end = sp - 1; /* name ends at '!'  */
+      while (*sp == '0' && sp < ep) sp++; /* skip leading zeroes */
+      if (*sp == '0') return (0); /* version 0 is not valid */
+      *v_start = sp;   /* version start after '!' */
+      *v_length = (ep - sp) + 1;
+      return ((*v_length >= VERSIONLEN) ? 0 : 1); /* fail on version too long */
+    }
+  }

+  /* if the name is too short to have a name and a version number
+     ".~#~" or doesn't end with "~" then there is no version number
+  */
+  if (name_len < 4 || *ep != '~')
+    return (0);

+  /* The name ends with a "~" so scan back in the filename looking for
+     another "~" terminating early if we need only digits and find
+     something else
+  */
+  sp = ep - 1;
+  while (sp > name && *sp != '~') {
+    if (digitsonly && !isdigit(*sp)) return (0);
+    --sp;
+  }

+  /* test for no initial "~" or no "." before "~", or
+   * version number length not at least 1
+   */
+  if (sp == name || *(sp - 1) != '.' || (ep - sp) - 1 < 1)
+    return (0);

+  /* After this point we have a version number in the form .~#~ with sp
+     pointing at the starting "~", ep pointing at the last "~",
+     and there must be at least one digit. Scan past any leading
+     zeros in the version, taking care not to remove the last digit.
+  */

+  *n_end = sp - 1;    /* save location of "." */
+
+  sp++;                  /* skip over the "." */
+  while (*sp == '0' && sp < (ep - 1)) {
+    sp++;
+  }
+  if (*sp == '0') return (0);  /* version 0 is not valid */
+  *v_start = sp;           /* save location of first significant digit in version */
+  *v_length = (ep - sp);   /* save length of version */
+  return ((*v_length >= VERSIONLEN) ? 0 : 1); /* fail on version too long */
+}

 /********************************************************/
 /*  file-system-specific defns                */
--- a/src/dsk.c
+++ b/src/dsk.c
@@ -3246,6 +3246,7 @@ static int maintain_version(char *file, int forcep)
     * is versioned one higher than the existing highest version.
     */
    FindHighestVersion(VA.files, entry, max_no);
+    if (max_no >= MAXVERSION) {*Lisp_errno = EIO; return (0);}
    sprintf(ver, "%u", max_no + 1);
 /*
 * The old file should have the same case name as the versionless
@@ -3570,6 +3571,7 @@ static int get_old(char *dir, FileName *varray, char *afile, char *vfile)
         * link missing versionless file.
         */
        FindHighestVersion(varray, entry, max_no);
+        if (max_no >= MAXVERSION) {*Lisp_errno = EIO; return (0);}
        sprintf(vbuf, "%u", max_no + 1);
        ConcNameAndVersion(vless, vbuf, vfile);
        strcpy(afile, vless);
@@ -3578,6 +3580,7 @@ static int get_old(char *dir, FileName *varray, char *afile, char *vfile)
        /* A version is specified. */
        ver_no = strtoul(ver, (char **)NULL, 10);
        FindHighestVersion(varray, entry, max_no);
+        if (max_no >= MAXVERSION) {*Lisp_errno = EIO; return (0);}
        if (ver_no == max_no + 1) {
          /*
           * If the version is one higher than the
@@ -3926,6 +3929,7 @@ static int get_new(char *dir, FileName *varray, char *afile, char *vfile)
       * the existing highest version.
       */
      FindHighestVersion(varray, entry, max_no);
+      if (max_no + 1 >= MAXVERSION) {*Lisp_errno = EIO; return (0);}
      sprintf(vbuf, "%u", max_no + 1);
      /*
       * We will use the file name of the existing highest
@@ -4018,6 +4022,7 @@ static int get_new(char *dir, FileName *varray, char *afile, char *vfile)
         * missing versionless file.
         */
        FindHighestVersion(varray, entry, max_no);
+        if (max_no + 1 >= MAXVERSION) {*Lisp_errno = EIO; return (0);}
        sprintf(vbuf, "%u", max_no + 2);
        ConcNameAndVersion(vless, vbuf, vfile);
        strcpy(afile, vfile);
@@ -4081,6 +4086,7 @@ static int get_new(char *dir, FileName *varray, char *afile, char *vfile)
         * new file.
         */
        FindHighestVersion(varray, entry, max_no);
+        if (max_no >= MAXVERSION) {*Lisp_errno = EIO; return (0);}
        sprintf(vbuf, "%u", max_no + 1);
        /*
         * We will use the name of the highest versioned file
@@ -4297,6 +4303,7 @@ static int get_old_new(char *dir, FileName *varray, char *afile, char *vfile)
         * link missing versionless file.
         */
        FindHighestVersion(varray, entry, max_no);
+        if (max_no >= MAXVERSION) {*Lisp_errno = EIO; return (0);}
        sprintf(vbuf, "%u", max_no + 1);
        ConcNameAndVersion(vless, vbuf, vfile);
        strcpy(afile, vless);
Author	SHA1	Message	Date
Nick Briggs	06399b0cf8	Declares correct size for scratch version string storage	2025-07-25 14:27:22 -07:00
Nick Briggs	88dea29837	Replaces LispStringToCString macro with static inline procedure	2025-07-25 14:27:22 -07:00
Nick Briggs	feac148fb4	Adds error returns for case where new file version would exceed MAXVERSION	2025-07-25 14:27:22 -07:00
Nick Briggs	a8f968f32d	Replaces ConcDirAndName and ConcNameAndVersion macros with static inline procedures. These are used in many places so code bloat can be reduced by using procedures.	2025-07-25 14:27:22 -07:00
Nick Briggs	5060cae998	Improves parsing of Unix format file names, updates comments * Adds a (static inline) helper, parse_file_version() to version parsing code. - Removes handling of "filename%" as version 0 (version 0 is not valid) - Adds handling of Alto/IFS version "filename!nnn" - Improves robustness of version parsing (no integer overflow errors) * Replaces private FNAMETOOLONG error code with ENAMETOOLONG standard error. * Replaces tests for "magic numbers" with standard ERRNO values (ENOENT, EXDEV) in error case for rename() operations. * Reduces storage requirement for versions from 16 bytes to 10 bytes to match the maximum acceptable version number, "999999999". * Updates various code comments to be correct and clearer.	2025-07-25 14:27:22 -07:00