Fix #344, Consistent directory entry size limit

Changes FileName in os_dirent_t from OS_MAX_PATH_LEN to OS_MAX_FILE_NAME,
and moves OS_check_name_length into OS_TranslatePath so it is
consistantly applied everywhere. Also fixes the length checks in
OS_check_name_length to account for terminating null.

Unit tests updated to match new directory name limit.

src/bsp/mcp750-vxworks/config/osconfig.h

@@ -42,7 +42,7 @@
 #define OS_MAX_API_NAME     20
 
 /* 
-** The maximum length for a file name 
+** The maximum length (including null terminator) for a file name
 */
 #define OS_MAX_FILE_NAME    20
 

src/bsp/pc-linux/config/osconfig.h

@@ -41,7 +41,7 @@
 #define OS_MAX_API_NAME     20
 
 /* 
-** The maximum length for a file name 
+** The maximum length (including null terminator) for a file name 
 */
 #define OS_MAX_FILE_NAME    20
 

src/bsp/pc-rtems/config/osconfig.h

@@ -41,7 +41,7 @@
 #define OS_MAX_API_NAME     20
 
 /* 
-** The maximum length for a file name 
+** The maximum length (including null terminator) for a file name 
 */
 #define OS_MAX_FILE_NAME    20
 

src/os/inc/osapi-os-filesys.h

@@ -189,7 +189,7 @@ enum
 /** @brief Directory entry */
 typedef struct
 {
-   char FileName[OS_MAX_PATH_LEN];
+   char FileName[OS_MAX_FILE_NAME];
 } os_dirent_t;
 
 #ifndef OSAL_OMIT_DEPRECATED

src/os/portable/os-impl-posix-dirs.c

@@ -133,8 +133,8 @@ int32 OS_DirRead_Impl(uint32 local_id, os_dirent_t *dirent)
       return OS_FS_ERROR;
    }
 
-   strncpy(dirent->FileName, de->d_name, OS_MAX_PATH_LEN - 1);
-   dirent->FileName[OS_MAX_PATH_LEN - 1] = 0;
+   strncpy(dirent->FileName, de->d_name, sizeof(dirent->FileName) - 1);
+   dirent->FileName[sizeof(dirent->FileName) - 1] = 0;
 
    return OS_FS_SUCCESS;
 } /* end OS_DirRead_Impl */

src/os/shared/osapi-file.c

@@ -53,41 +53,7 @@ enum
 };
 
 OS_stream_internal_record_t         OS_stream_table[OS_MAX_NUM_OPEN_FILES];
-
 
-/*----------------------------------------------------------------
- *
- * Function: OS_check_name_length
- *
- *  Purpose: Local helper routine, not part of OSAL API.
- *           Validates that the path length is within spec and
- *           contains at least one directory separator (/) char.
- *
- *-----------------------------------------------------------------*/
-static int32 OS_check_name_length(const char *path)
-{
-    char* name_ptr;
-
-    if (path == NULL)
-        return OS_FS_ERR_INVALID_POINTER;
-
-    if (strlen(path) > OS_MAX_PATH_LEN)
-        return OS_FS_ERR_PATH_TOO_LONG;
-
-    /* checks to see if there is a '/' somewhere in the path */
-    name_ptr = strrchr(path, '/');
-    if (name_ptr == NULL)
-        return OS_FS_ERROR;
-
-    /* strrchr returns a pointer to the last '/' char, so we advance one char */
-    name_ptr = name_ptr + 1;
-
-    if( strlen(name_ptr) > OS_MAX_FILE_NAME)
-        return OS_FS_ERR_NAME_TOO_LONG;
-
-    return OS_FS_SUCCESS;
-
-} /* end OS_check_name_length */
 
 /****************************************************************************************
                                   FILE API
@@ -126,16 +92,9 @@ static int32 OS_OpenCreate(uint32 *filedes, const char *path, int32 flags, int32
    char   local_path[OS_MAX_LOCAL_PATH_LEN];
 
    /*
-   ** check if the name of the file is too long
-   */
-   return_code = OS_check_name_length(path);
-   if (return_code == OS_FS_SUCCESS)
-   {
-      /*
-      ** Translate the path
-      */
-      return_code = OS_TranslatePath(path, (char *)local_path);
-   }
+    * Translate the path
+    */
+   return_code = OS_TranslatePath(path, (char *)local_path);
 
    if (return_code == OS_FS_SUCCESS)
    {
@@ -455,14 +414,10 @@ int32 OS_remove (const char *path)
    int32 return_code;
    char local_path[OS_MAX_LOCAL_PATH_LEN];
 
-   return_code = OS_check_name_length(path);
+   return_code = OS_TranslatePath(path, local_path);
    if (return_code == OS_FS_SUCCESS)
    {
-      return_code = OS_TranslatePath(path, local_path);
-      if (return_code == OS_FS_SUCCESS)
-      {
-         return_code = OS_FileRemove_Impl(local_path);
-      }
+      return_code = OS_FileRemove_Impl(local_path);
    }
 
    return return_code;
@@ -485,15 +440,7 @@ int32 OS_rename (const char *old, const char *new)
    char old_path[OS_MAX_LOCAL_PATH_LEN];
    char new_path[OS_MAX_LOCAL_PATH_LEN];
 
-   return_code = OS_check_name_length(old);
-   if (return_code == OS_FS_SUCCESS)
-   {
-      return_code = OS_check_name_length(new);
-   }
-   if (return_code == OS_FS_SUCCESS)
-   {
-      return_code = OS_TranslatePath(old, old_path);
-   }
+   return_code = OS_TranslatePath(old, old_path);
    if (return_code == OS_FS_SUCCESS)
    {
       return_code = OS_TranslatePath(new, new_path);

src/os/shared/osapi-filesys.c

@@ -66,6 +66,41 @@ extern const OS_VolumeInfo_t OS_VolumeTable[];
 #define OS_COMPAT_VOLTAB_SIZE       0
 #endif
 
+
+/*----------------------------------------------------------------
+ *
+ * Function: OS_check_name_length
+ *
+ *  Purpose: Local helper routine, not part of OSAL API.
+ *           Validates that the path length is within spec and
+ *           contains at least one directory separator (/) char.
+ *
+ *-----------------------------------------------------------------*/
+static int32 OS_check_name_length(const char *path)
+{
+    char* name_ptr;
+
+    if (path == NULL)
+        return OS_FS_ERR_INVALID_POINTER;
+
+    if (strlen(path) >= OS_MAX_PATH_LEN)
+        return OS_FS_ERR_PATH_TOO_LONG;
+
+    /* checks to see if there is a '/' somewhere in the path */
+    name_ptr = strrchr(path, '/');
+    if (name_ptr == NULL)
+        return OS_FS_ERR_PATH_INVALID;
+
+    /* strrchr returns a pointer to the last '/' char, so we advance one char */
+    name_ptr = name_ptr + 1;
+
+    if( strlen(name_ptr) >= OS_MAX_FILE_NAME)
+        return OS_FS_ERR_NAME_TOO_LONG;
+
+    return OS_FS_SUCCESS;
+
+} /* end OS_check_name_length */
+
 
 /*----------------------------------------------------------------
  *
@@ -1061,18 +1096,19 @@ int32 OS_TranslatePath(const char *VirtualPath, char *LocalPath)
         return OS_FS_ERR_INVALID_POINTER;
     }
 
-    SysMountPointLen = 0;
-    VirtPathLen = strlen(VirtualPath);
-    VirtPathBegin = VirtPathLen;
-
     /*
-    ** Check to see if the path is too long
+    ** Check length
     */
-    if (VirtPathLen >= OS_MAX_PATH_LEN)
+    return_code = OS_check_name_length(VirtualPath);
+    if (return_code != OS_SUCCESS)
     {
-        return OS_FS_ERR_PATH_TOO_LONG;
+        return return_code;
     }
 
+    SysMountPointLen = 0;
+    VirtPathLen = strlen(VirtualPath);
+    VirtPathBegin = VirtPathLen;
+
     /*
     ** All valid Virtual paths must start with a '/' character
     */

src/os/vxworks/osfileapi.c

@@ -180,8 +180,8 @@ int32 OS_DirRead_Impl(uint32 local_id, os_dirent_t *dirent)
       return OS_FS_ERROR;
    }
 
-   strncpy(dirent->FileName, de->d_name, OS_MAX_PATH_LEN - 1);
-   dirent->FileName[OS_MAX_PATH_LEN - 1] = 0;
+   strncpy(dirent->FileName, de->d_name, sizeof(dirent->FileName) - 1);
+   dirent->FileName[sizeof(dirent->FileName) - 1] = 0;
 
    return OS_FS_SUCCESS;
 } /* end OS_DirRead_Impl */

src/tests/file-api-test/file-api-test.c

@@ -95,55 +95,65 @@ void TestCreatRemove(void)
     char maxfilename[OS_MAX_PATH_LEN];
     char longfilename [OS_MAX_PATH_LEN + 10];
     int  status;
+    int  fd;
+    int  i;
 
-    strncpy(filename,"/drive0/test11chars", sizeof(filename) - 1);
-    filename[sizeof(filename) - 1] = 0;
+    /* Short file name */
+    strncpy(filename,"/drive0/a", sizeof(filename));
 
-    /* make the two really long filenames */
-
-    /* 1 '/' and 40 'm' */
-    strncpy(maxfilename,"/drive0/mmmmmmmmmmmmmmmmmmmm", sizeof(maxfilename) - 1);
-    maxfilename[sizeof(maxfilename) - 1] = 0;
+    /* Create max file name (OS_MAX_FILE_NAME including terminating null) */
+    strncpy(maxfilename,"/drive0/", sizeof(maxfilename));
+    for (i = strlen(maxfilename); i < (OS_MAX_FILE_NAME - 1); i++)
+    {
+        maxfilename[i] = 'm';
+    }
 
-    /* 1 '/' and 41 'l' */
-    strncpy(longfilename,"/drive0/lllllllllllllllllllllllllllllllllllllllll", sizeof(longfilename) - 1);
-    longfilename[sizeof(longfilename) - 1] = 0;
+    /* Create longer than max file name */
+    strncpy(longfilename,"/drive0/", sizeof(longfilename));
+    for (i = strlen(longfilename); i < (sizeof(longfilename) - 1); i++)
+    {
+        longfilename[i] = 'm';
+    }
 
-    /* create a file of reasonable length (but over 8 chars) */
-    status = OS_creat(filename,OS_READ_WRITE);
-    UtAssert_True(status >= OS_SUCCESS, "status after creat = %d",(int)status);
+    /* create a file with short name */
+    fd = OS_creat(filename,OS_READ_WRITE);
+    UtAssert_True(fd >= 0, "fd after creat short name length file = %d",(int)fd);
 
     /* close the first file */
-    status = OS_close(status);
-    UtAssert_True(status == OS_SUCCESS, "status after close = %d",(int)status);
+    status = OS_close(fd);
+    UtAssert_True(status == OS_SUCCESS, "status after close short name length file = %d",(int)status);
 
-    /* create a file of OS_max_file_name size */
-    status = OS_creat(maxfilename,OS_READ_WRITE);
-    UtAssert_True(status >= OS_SUCCESS, "status after creat = %d",(int)status);
+    /* create a file with max name size */
+    fd = OS_creat(maxfilename,OS_READ_WRITE);
+    UtAssert_True(fd >= 0, "fd after creat max name length file = %d",(int)fd);
 
     /* close the second file */
-    status = OS_close(status);
-    UtAssert_True(status == OS_SUCCESS, "status after close = %d",(int)status);
+    status = OS_close(fd);
+    UtAssert_True(status == OS_SUCCESS, "status after close max name length file = %d",(int)status);
 
-    /* try removing the file from the drive */
+    /* remove the file from the drive */
     status = OS_remove(filename);
-    UtAssert_True(status == OS_SUCCESS, "status after remove = %d",(int)status);
+    UtAssert_True(status == OS_SUCCESS, "status after short name length file remove = %d",(int)status);
 
-    /* try removing the file from the drive */
+    /* remove the file from the drive */
     status = OS_remove(maxfilename);
-    UtAssert_True(status == OS_SUCCESS, "status after remove = %d",(int)status);
+    UtAssert_True(status == OS_SUCCESS, "status after remove max name length file = %d",(int)status);
 
-    /* try removing the file from the drive. Should Fail */
+    /* try creating with file name too big, should fail */
+    status = OS_creat(longfilename,OS_READ_WRITE);
+    UtAssert_True(status < OS_SUCCESS, "status after create file name too long = %d",(int)status);
+
+    /* try removing with file name too big. Should Fail */
     status = OS_remove(longfilename);
-    UtAssert_True(status < OS_SUCCESS, "status after remove = %d",(int)status);
+    UtAssert_True(status < OS_SUCCESS, "status after remove file name too long = %d",(int)status);
 
-    /* try removing the file from the drive. Should Fail */
-    status = OS_remove("/drive0/FileNotOnDrive");
-    UtAssert_True(status < OS_SUCCESS, "status after remove = %d",(int)status);
+    /* try removing file that no longer exists. Should Fail */
+    status = OS_remove(filename);
+    UtAssert_True(status < OS_SUCCESS, "status after remove file doesn't exist = %d",(int)status);
 
     /* Similar to previous but with a bad mount point (gives different error code) */
-    status = OS_remove("/FileNotOnDrive");
-    UtAssert_True(status == OS_FS_ERR_PATH_INVALID, "status after remove = %d",(int)status);
+    status = OS_remove("/x");
+    UtAssert_True(status == OS_FS_ERR_PATH_INVALID, "status after remove bad mount = %d",(int)status);
 }
 
 /*---------------------------------------------------------------------------------------
@@ -781,13 +791,16 @@ void TestStat(void)
 
     char        filename1[OS_MAX_PATH_LEN];
     char        dir1 [OS_MAX_PATH_LEN];
+    char        dir1slash[OS_MAX_PATH_LEN];
     char        buffer1 [OS_MAX_PATH_LEN];
     os_fstat_t  StatBuff;
     int32       fd1;
     int         size;
 
-    strcpy(dir1,"/drive0/ThisIsALongDirectoryName");
-    strcpy(filename1,"/drive0/ThisIsALongDirectoryName/MyFile1");
+    strcpy(dir1,"/drive0/DirectoryName");
+    strcpy(dir1slash, dir1);
+    strcat(dir1slash, "/");
+    strcpy(filename1,"/drive0/DirectoryName/MyFile1");
     strcpy(buffer1,"111111111111");
 
     /* make the directory */
@@ -812,13 +825,13 @@ void TestStat(void)
     /* 
     ** Make the stat calls 
     */
-    status = OS_stat( "/drive0/ThisIsALongDirectoryName/",&StatBuff);
+    status = OS_stat(dir1slash,&StatBuff);
     UtAssert_True(status == OS_SUCCESS, "status after stat 1 = %d",(int)status);
 
-    status = OS_stat( "/drive0/ThisIsALongDirectoryName",&StatBuff);
+    status = OS_stat(dir1,&StatBuff);
     UtAssert_True(status == OS_SUCCESS, "status after stat 2 = %d",(int)status);
 
-    status = OS_stat( "/drive0/ThisIsALongDirectoryName/MyFile1",&StatBuff);
+    status = OS_stat(filename1,&StatBuff);
     UtAssert_True(status == OS_SUCCESS, "status after stat 3 = %d",(int)status);
 
     /* Clean up */