From 52f64201b0eec92d4a2580ec94981dc15c1549a8 Mon Sep 17 00:00:00 2001 From: Nick Briggs Date: Thu, 26 Dec 2024 14:56:48 -0800 Subject: [PATCH 1/5] 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. --- inc/locfile.h | 170 ++++++++++++++++++++++++++++---------------------- 1 file changed, 94 insertions(+), 76 deletions(-) diff --git a/inc/locfile.h b/inc/locfile.h index 856c23d6..9e450130 100644 --- a/inc/locfile.h +++ b/inc/locfile.h @@ -8,8 +8,11 @@ /* Manufactured in the United States of America. */ /* */ /************************************************************************/ +#include /* for isdigit */ #include #include /* for NAME_MAX */ +#include /* for strlen */ +#include /* for MAXPATHLEN */ #include /* for MAXNAMLEN */ #include "lispemul.h" /* for DLword */ @@ -331,14 +334,10 @@ do { \ /* * 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: @@ -353,62 +352,20 @@ do { \ * 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 @@ -481,9 +438,12 @@ 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 { \ +#define ConcNameAndVersion(name, ver, rname) do { \ if (*(ver) != '\0') { \ strcpy(rname, name); \ strcat(rname, ".~"); \ @@ -494,7 +454,7 @@ do { \ } \ } while (0) -#define VERSIONLEN 16 +#define VERSIONLEN 10 #define MAXVERSION 999999999 @@ -576,9 +536,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 +561,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 */ From ca98cc2606609475d95bb7cc29d4e73d6846997e Mon Sep 17 00:00:00 2001 From: Nick Briggs Date: Thu, 2 Jan 2025 15:17:42 -0800 Subject: [PATCH 2/5] Replaces ConcDirAndName and ConcNameAndVersion macros with static inline procedures. These are used in many places so code bloat can be reduced by using procedures. --- inc/locfile.h | 107 +++++++++++++++++++++++++------------------------- 1 file changed, 54 insertions(+), 53 deletions(-) diff --git a/inc/locfile.h b/inc/locfile.h index 9e450130..9cba3709 100644 --- a/inc/locfile.h +++ b/inc/locfile.h @@ -296,7 +296,7 @@ do { \ goto truetag; /* NOLINT(bugprone-macro-parentheses) */ \ } while (0) -/* +/* * Name: LispVersionToUnixVersion * * Argument: char *pathname @@ -331,7 +331,7 @@ do { \ #endif /* DOS */ -/* +/* * Name: UnixVersionToLispVersion * * Argument: char *pathname UNIX syntax pathname. @@ -349,7 +349,7 @@ 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) \ @@ -367,7 +367,7 @@ do { \ } \ } while (0) -/* +/* * Name: ConcDirAndName * * Argument: char *dir The name of the directory. @@ -385,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. @@ -443,16 +443,17 @@ do { \ * */ -#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 10 @@ -502,7 +503,7 @@ do { \ *(lf_cp-1) = '\0'; \ } \ } while (0) - + #define ChangeToVersionless(pathname) do { \ char *lf_cp; \ if( (lf_cp=strrchr(pathname, ';')) != 0) \ From d6e2e12df589c10c8c541ab5221df7e471002b2e Mon Sep 17 00:00:00 2001 From: Nick Briggs Date: Thu, 2 Jan 2025 15:24:52 -0800 Subject: [PATCH 3/5] Adds error returns for case where new file version would exceed MAXVERSION --- src/dsk.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/dsk.c b/src/dsk.c index 75d9f4c8..f97127b7 100644 --- a/src/dsk.c +++ b/src/dsk.c @@ -3243,6 +3243,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 @@ -3567,6 +3568,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); @@ -3575,6 +3577,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 @@ -3923,6 +3926,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 @@ -4015,6 +4019,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); @@ -4078,6 +4083,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 @@ -4294,6 +4300,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); From d5430c2b0edc23109be8d81fab92df81d3d9de5f Mon Sep 17 00:00:00 2001 From: Nick Briggs Date: Thu, 2 Jan 2025 17:57:21 -0800 Subject: [PATCH 4/5] Replaces LispStringToCString macro with static inline procedure --- inc/locfile.h | 124 +++++++++++++++++++++++++------------------------- 1 file changed, 62 insertions(+), 62 deletions(-) diff --git a/inc/locfile.h b/inc/locfile.h index 9cba3709..0d90baae 100644 --- a/inc/locfile.h +++ b/inc/locfile.h @@ -15,6 +15,7 @@ #include /* for MAXPATHLEN */ #include /* for MAXNAMLEN */ #include "lispemul.h" /* for DLword */ +#include "commondefs.h" /* for error */ #define FDEV_PAGE_SIZE 512 /* 1 page == 512 byte */ @@ -66,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 */ /************************************************************************/ /* */ @@ -82,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 */ @@ -192,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) : \ From 6193f2733fd6b32eea3eed8fe08dfa2cabc22ff6 Mon Sep 17 00:00:00 2001 From: Nick Briggs Date: Fri, 3 Jan 2025 09:05:12 -0800 Subject: [PATCH 5/5] Declares correct size for scratch version string storage --- inc/lispver2.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inc/lispver2.h b/inc/lispver2.h index 7e180091..a1fa6ed5 100644 --- 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; \