cadius icon indicating copy to clipboard operation
cadius copied to clipboard

Failure allocating blocks for file when last block is used

Open inexorabletash opened this issue 2 years ago • 0 comments

When adding a file and the last block on the volume will be used, the block allocation fails and a bogus index file is created.

Repro:

  • Create a 140k floppy volume.
  • Consume all but 21 blocks
  • Call ADDFILE with BASIC.SYSTEM

Expected:

  • Sapling file entry created showing 20 blocks used, with 1 index block. Volume has 0 blocks free.

Actual:

  • Sapling file entry created showing 20 blocks used, with 1 index block. Block contents are null. Volume has 20 blocks free.

Root cause:

In AllocateImageBlock, a loop searches for the first free block in the volume table. Given the above example, it's trying to find 20 blocks on a 280 block volume. It should search up to and including block 260, since it can use blocks 260...279 to hold a 20 block file.

The current code stops after considering block 259 - block 260 is not considered. A classic off-by-one. Here's a fix:

diff --git a/Src/Dc_Prodos.c b/Src/Dc_Prodos.c
index 1747566..40a83bf 100644
--- a/Src/Dc_Prodos.c
+++ b/Src/Dc_Prodos.c
@@ -1974,7 +1974,7 @@ int *AllocateImageBlock(struct prodos_image *current_image, int nb_block)
     }
 
   /** 1ère passe, on recherche les X blocs consécutifs **/
-  for(i=0,nb_free_block=0; i<current_image->nb_block-nb_block; i++)
+  for(i=0,nb_free_block=0; i<=current_image->nb_block-nb_block; i++)
     {
       if(current_image->block_allocation_table[i] == 1)
         {

It's also concerning that Cadius doesn't fail at this point. Instead, the code sets nb_free_block and first_free_block to 0, and the rest of the code allocates/returns a 20 entry block table filled with 0 for block numbers. This causes the corrupt image to be created.

inexorabletash avatar Jul 05 '22 18:07 inexorabletash