dosfs: Fix fat_file_write()

Remove forced overwrite which leads to file data corruption.  The logic
to determine a forced overwrite was fundamentally broken.  For simplity,
disable this feature.

Close #2622.

cpukit/libfs/src/dosfs/fat.c

@@ -200,13 +200,12 @@ _fat_block_read(
 }
 
 static ssize_t
- fat_block_write(
+fat_block_write(
     fat_fs_info_t                        *fs_info,
     const uint32_t                        start_blk,
     const uint32_t                        offset,
     const uint32_t                        count,
-    const void                           *buf,
+    const void                           *buf)
-    const bool                            overwrite_block)
 {
     int                 rc             = RC_OK;
     uint32_t            bytes_to_write = MIN(count, (fs_info->vol.bytes_per_block - offset));
@@ -215,8 +214,7 @@ static ssize_t
 
     if (0 < bytes_to_write)
     {
-        if (   overwrite_block
+        if (bytes_to_write == fs_info->vol.bytes_per_block)
-            || (bytes_to_write == fs_info->vol.bytes_per_block))
         {
             rc = fat_buf_access(fs_info, sec_num, FAT_OP_TYPE_GET, &blk_buf);
         }
@@ -399,7 +397,6 @@ _fat_block_release(fat_fs_info_t *fs_info)
  *     offset             - offset inside cluster 'start'
  *     count              - count of bytes to write
  *     buff               - buffer provided by user
- *     overwrite_cluster  - true if cluster can get overwritten, false if cluster content must be kept
  *
  * RETURNS:
  *     bytes written on success, or -1 if error occured
@@ -411,8 +408,7 @@ fat_cluster_write(
     const uint32_t                        start_cln,
     const uint32_t                        offset,
     const uint32_t                        count,
-    const void                           *buff,
+    const void                           *buff)
-    const bool                            overwrite_cluster)
 {
     ssize_t             rc               = RC_OK;
     uint32_t            bytes_to_write   = MIN(count, (fs_info->vol.bpc - offset));
@@ -436,8 +432,7 @@ fat_cluster_write(
           cur_blk,
           ofs_blk,
           c,
-          &buffer[bytes_written],
+          &buffer[bytes_written]);
-          overwrite_cluster);
       if (c != ret)
         rc = -1;
       else

cpukit/libfs/src/dosfs/fat.h

@@ -511,8 +511,7 @@ fat_cluster_write(fat_fs_info_t                    *fs_info,
                     uint32_t                          start_cln,
                     uint32_t                          offset,
                     uint32_t                          count,
-                    const void                       *buff,
+                    const void                       *buff);
-                    bool                              overwrite_cluster);
 
 ssize_t
 fat_sector_write(fat_fs_info_t                        *fs_info,

cpukit/libfs/src/dosfs/fat_file.c

@@ -403,20 +403,18 @@ static bool
  *     start            - offset(in bytes) to write from
  *     count            - count
  *     buf              - buffer provided by user
- *     file_cln_initial - initial current cluster number of the file
  *
  * RETURNS:
  *     number of bytes actually written to the file on success, or -1 if
  *     error occured (errno set appropriately)
  */
 static ssize_t
- fat_file_write_fat32_or_non_root_dir(
+fat_file_write_fat32_or_non_root_dir(
      fat_fs_info_t                        *fs_info,
      fat_file_fd_t                        *fat_fd,
      const uint32_t                        start,
      const uint32_t                        count,
-     const uint8_t                        *buf,
+     const uint8_t                        *buf)
-     const uint32_t                        file_cln_initial)
 {
     int            rc = RC_OK;
     uint32_t       cmpltd = 0;
@@ -426,35 +424,27 @@ static ssize_t
     uint32_t       ofs_cln = start - (start_cln << fs_info->vol.bpc_log2);
     uint32_t       ofs_cln_save = ofs_cln;
     uint32_t       bytes_to_write = count;
-    uint32_t       file_cln_cnt;
     ssize_t        ret;
     uint32_t       c;
-    bool           overwrite_cluster = false;
 
     rc = fat_file_lseek(fs_info, fat_fd, start_cln, &cur_cln);
     if (RC_OK == rc)
     {
-        file_cln_cnt = cur_cln - fat_fd->cln;
         while (   (RC_OK == rc)
                && (bytes_to_write > 0))
         {
             c = MIN(bytes_to_write, (fs_info->vol.bpc - ofs_cln));
 
-            if (file_cln_initial < file_cln_cnt)
-                overwrite_cluster = true;
-
             ret = fat_cluster_write(fs_info,
                                       cur_cln,
                                       ofs_cln,
                                       c,
-                                      &buf[cmpltd],
+                                      &buf[cmpltd]);
-                                      overwrite_cluster);
             if (0 > ret)
               rc = -1;
 
             if (RC_OK == rc)
             {
-                ++file_cln_cnt;
                 bytes_to_write -= ret;
                 cmpltd += ret;
                 save_cln = cur_cln;
@@ -509,7 +499,6 @@ fat_file_write(
     uint32_t       byte;
     uint32_t       c = 0;
     bool           zero_fill = start > fat_fd->fat_file_size;
-    uint32_t       file_cln_initial = fat_fd->map.file_cln;
     uint32_t       cln;
 
 
@@ -543,8 +532,7 @@ fat_file_write(
                                       cln,
                                       byte,
                                       count,
-                                      buf,
+                                      buf);
-                                      false);
             if (0 > ret)
               rc = -1;
             else
@@ -556,8 +544,7 @@ fat_file_write(
                                                        fat_fd,
                                                        start,
                                                        count,
-                                                       buf,
+                                                       buf);
-                                                       file_cln_initial);
             if (0 > ret)
               rc = -1;
             else

testsuites/fstests/Makefile.am

@@ -5,6 +5,7 @@ _SUBDIRS += fsimfsconfig03
 _SUBDIRS += fsimfsconfig02
 _SUBDIRS += fsimfsconfig01
 _SUBDIRS += fsdosfsname01
+_SUBDIRS += fsdosfsname02
 _SUBDIRS += fsdosfswrite01
 _SUBDIRS += fsdosfsformat01
 _SUBDIRS += fsfseeko01

testsuites/fstests/configure.ac

@@ -81,6 +81,7 @@ fsimfsconfig03/Makefile
 fsimfsconfig02/Makefile
 fsimfsconfig01/Makefile
 fsdosfsname01/Makefile
+fsdosfsname02/Makefile
 fsdosfswrite01/Makefile
 fsdosfsformat01/Makefile
 fsfseeko01/Makefile

testsuites/fstests/fsdosfsname02/Makefile.am

@@ -0,0 +1,19 @@
+rtems_tests_PROGRAMS = fsdosfsname02
+fsdosfsname02_SOURCES = init.c
+
+dist_rtems_tests_DATA = fsdosfsname02.scn fsdosfsname02.doc
+
+include $(RTEMS_ROOT)/make/custom/@RTEMS_BSP@.cfg
+include $(top_srcdir)/../automake/compile.am
+include $(top_srcdir)/../automake/leaf.am
+
+AM_CPPFLAGS += -I$(top_srcdir)/../support/include
+
+LINK_OBJS = $(fsdosfsname02_OBJECTS)
+LINK_LIBS = $(fsdosfsname02_LDLIBS)
+
+fsdosfsname02$(EXEEXT): $(fsdosfsname02_OBJECTS) $(fsdosfsname02_DEPENDENCIES)
+	@rm -f fsdosfsname02$(EXEEXT)
+	$(make-exe)
+
+include $(top_srcdir)/../automake/local.am

testsuites/fstests/fsdosfsname02/fsdosfsname02.doc

@@ -0,0 +1,11 @@
+This file describes the directives and concepts tested by this test set.
+
+test set name: fsdosfsname02
+
+directives:
+
+  - msdos_add_file()
+
+concepts:
+
+  - Ensure that directory entries accross cluster boundaries work.

testsuites/fstests/fsdosfsname02/fsdosfsname02.scn

@@ -0,0 +1,2 @@
+*** BEGIN OF TEST FSDOSFSNAME 2 ***
+*** END OF TEST FSDOSFSNAME 2 ***

testsuites/fstests/fsdosfsname02/init.c

@@ -0,0 +1,118 @@
+/*
+ * Copyright (c) 2017 embedded brains GmbH.  All rights reserved.
+ *
+ *  embedded brains GmbH
+ *  Dornierstr. 4
+ *  82178 Puchheim
+ *  Germany
+ *  <rtems@embedded-brains.de>
+ *
+ * The license and distribution terms for this file may be
+ * found in the file LICENSE in this distribution or at
+ * http://www.rtems.org/license/LICENSE.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include <rtems/dosfs.h>
+#include <rtems/ramdisk.h>
+
+#include "tmacros.h"
+
+const char rtems_test_name[] = "FSDOSFSNAME 2";
+
+#define RAMDISK_PATH "/dev/rda"
+
+#define MOUNT_PATH "/mnt"
+
+static const char * const dir_paths[] = {
+  MOUNT_PATH "/cpukit",
+  MOUNT_PATH "/cpukit/or1k-exception-frame-print.c",
+  MOUNT_PATH "/cpukit/preinstall.am",
+  MOUNT_PATH "/cpukit/Makefile.in",
+  MOUNT_PATH "/cpukit/Makefile.am",
+  MOUNT_PATH "/cpukit/rtems",
+  MOUNT_PATH "/cpukit/or1k-context-switch.S",
+  MOUNT_PATH "/cpukit/or1k-exception-default.c",
+  MOUNT_PATH "/cpukit/or1k-context-initialize.c",
+  MOUNT_PATH "/cpukit/or1k-context-volatile-clobber.S",
+  MOUNT_PATH "/cpukit/or1k-context-validate.S",
+  MOUNT_PATH "/cpukit/or1k-exception-handler-low.S",
+  MOUNT_PATH "/cpukit/cpu.c"
+};
+
+static void test(void)
+{
+  int rv;
+  size_t i;
+
+  rv = msdos_format(RAMDISK_PATH, NULL);
+  rtems_test_assert(rv == 0);
+
+  rv = mount_and_make_target_path(
+    RAMDISK_PATH,
+    MOUNT_PATH,
+    RTEMS_FILESYSTEM_TYPE_DOSFS,
+    RTEMS_FILESYSTEM_READ_WRITE,
+    NULL
+  );
+  rtems_test_assert(rv == 0);
+
+  for (i = 0; i < RTEMS_ARRAY_SIZE(dir_paths); ++i) {
+    rv = mkdir(dir_paths[i], S_IRWXU | S_IRWXG | S_IRWXO);
+    rtems_test_assert(rv == 0);
+  }
+
+  for (i = RTEMS_ARRAY_SIZE(dir_paths); i > 0; --i) {
+    rv = unlink(dir_paths[i - 1]);
+    rtems_test_assert(rv == 0);
+  }
+}
+
+static void Init(rtems_task_argument arg)
+{
+  TEST_BEGIN();
+
+  test();
+
+  TEST_END();
+  rtems_test_exit(0);
+}
+
+rtems_ramdisk_config rtems_ramdisk_configuration[] = {
+  { .block_size = 512, .block_num = 64 }
+};
+
+size_t rtems_ramdisk_configuration_size = RTEMS_ARRAY_SIZE(rtems_ramdisk_configuration);
+
+#define CONFIGURE_APPLICATION_DOES_NOT_NEED_CLOCK_DRIVER
+#define CONFIGURE_APPLICATION_NEEDS_CONSOLE_DRIVER
+#define CONFIGURE_APPLICATION_NEEDS_LIBBLOCK
+
+#define CONFIGURE_APPLICATION_EXTRA_DRIVERS RAMDISK_DRIVER_TABLE_ENTRY
+
+#define CONFIGURE_FILESYSTEM_DOSFS
+
+#define CONFIGURE_LIBIO_MAXIMUM_FILE_DESCRIPTORS 32
+
+#define CONFIGURE_BDBUF_CACHE_MEMORY_SIZE 512
+#define CONFIGURE_BDBUF_BUFFER_MIN_SIZE 512
+#define CONFIGURE_BDBUF_BUFFER_MAX_SIZE 512
+
+#define CONFIGURE_MAXIMUM_TASKS 1
+#define CONFIGURE_MAXIMUM_SEMAPHORES 1
+
+#define CONFIGURE_INITIAL_EXTENSIONS RTEMS_TEST_INITIAL_EXTENSION
+
+#define CONFIGURE_RTEMS_INIT_TASKS_TABLE
+
+#define CONFIGURE_INIT
+
+#include <rtems/confdefs.h>

testsuites/fstests/fsdosfswrite01/init.c

@@ -125,7 +125,7 @@ static void test_normal_file_write(
   const char *mount_dir,
   const char *file_name )
 {
-  static const rtems_blkdev_stats complete_block_stats = {
+  static const rtems_blkdev_stats complete_existing_block_stats = {
     .read_hits            = 0,
     .read_misses          = 0,
     .read_ahead_transfers = 0,
@@ -135,14 +135,24 @@ static void test_normal_file_write(
     .write_blocks         = 1,
     .write_errors         = 0
   };
-  static const rtems_blkdev_stats new_block_stats = {
+  static const rtems_blkdev_stats complete_new_block_stats = {
-    .read_hits            = 8,
+    .read_hits            = 3,
     .read_misses          = 2,
     .read_ahead_transfers = 0,
     .read_blocks          = 2,
     .read_errors          = 0,
     .write_transfers      = 1,
-    .write_blocks         = 4,
+    .write_blocks         = 3,
+    .write_errors         = 0
+  };
+  static const rtems_blkdev_stats partial_new_block_stats = {
+    .read_hits            = 3,
+    .read_misses          = 3,
+    .read_ahead_transfers = 0,
+    .read_blocks          = 3,
+    .read_errors          = 0,
+    .write_transfers      = 1,
+    .write_blocks         = 3,
     .write_errors         = 0
   };
 
@@ -174,17 +184,21 @@ static void test_normal_file_write(
   num_bytes = write( fd, cluster_buf, cluster_size );
   rtems_test_assert( (ssize_t) cluster_size == num_bytes );
 
-  check_block_stats( dev_name, mount_dir, &complete_block_stats );
+  check_block_stats( dev_name, mount_dir, &complete_existing_block_stats );
   reset_block_stats( dev_name, mount_dir );
 
+  /* Write a complete cluster into a new file space */
   num_bytes = write( fd, cluster_buf, cluster_size );
   rtems_test_assert( (ssize_t) cluster_size == num_bytes );
 
+  check_block_stats( dev_name, mount_dir, &complete_new_block_stats );
+  reset_block_stats( dev_name, mount_dir );
+
   /* Write a new partial cluster into a new file space */
   num_bytes = write( fd, cluster_buf, 1 );
   rtems_test_assert( num_bytes == 1 );
 
-  check_block_stats( dev_name, mount_dir, &new_block_stats );
+  check_block_stats( dev_name, mount_dir, &partial_new_block_stats );
 
   rv = close( fd );
   rtems_test_assert( 0 == rv );