Fix nasa#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 */