Page 1 of 1

Oricutron: BUG with .tap files

Posted: Tue Jul 14, 2015 3:09 pm
by christian
Hi Xeron,

I think i've found 2 bugs in oricutron both located in tape.c file:

First bug:
in function tape_load_tap

Code: Select all

  // TAP
  else if (memcmp(oric->tapebuf, "\x16\x16\x16", 3) == 0)
  {
    oric->rawtape = SDL_FALSE;

    // I give up trying to do anything clever.
    // Just allow an extra byte for broken tape images.
    oric->tapelen++;
  }
This try to correct bad .tap file (with the missing byte at the end of file), but later in function tape_patches

Code: Select all

        if( ( oric->cpu.read( &oric->cpu, oric->pch_fd_getname_addr ) != 0 ) &&
            ( oric->autoinsert ) )
        {
          // Only do this if there is no tape inserted, or we're at the
          // end of the current tape, or the filename ends in .TAP, .ORT or .WAV
          if( ( !oric->tapebuf ) ||
              ( oric->tapeoffs >= oric->tapelen ) ||
              ( ( i > 3 ) && ( strcasecmp( &oric->lasttapefile[i-4], ".tap" ) == 0 ) ) ||
              ( ( i > 3 ) && ( strcasecmp( &oric->lasttapefile[i-4], ".ort" ) == 0 ) ) ||
              ( ( i > 3 ) && ( strcasecmp( &oric->lasttapefile[i-4], ".wav" ) == 0 ) ) )
          {
            tape_autoinsert( oric );
            oric->cpu.write( &oric->cpu, oric->pch_fd_getname_addr, 0 );
          }
        }
      }
the test oric->tapeoffs >= oric->tapelen is never satisfied and the BASIC still wait the last byte.
One workaround is replace this test with oric->tapeoffs >= oric-tapelen-1
(or we have to specify the .TAP extension in every BASIC instruction which triggers ( i > 3 ) && ( strcasecmp( &oric->lasttapefile[i-4], ".tap" ) == 0 ) ) and force tape file load)

Don't know if there are some drawbacks with this change.

Second bug:
  1. Put 2 tapes in the tapes directory, say PROG1.TAP and PROG2.TAP.
  2. Start oricutron
  3. CLOAD "PROG1"
  4. CLOAD "PROG2"
-> The second file is never loaded.

The bug is in tape_autoinsert() function:

Code: Select all

void tape_autoinsert( struct machine *oric )
{
  char *odir;
  int i;

  if( strncmp( (char *)&oric->mem[oric->pch_fd_getname_addr], oric->lasttapefile, 16 ) == 0 )
    oric->mem[oric->pch_fd_getname_addr] = 0;

  // Try and load the tape image
  strcpy( tapefile, oric->lasttapefile );
  i = (int)strlen(tapefile);

  odir = getcwd( NULL, 0 );
  chdir( tapepath );
  tape_load_tap( oric, tapefile );
  if( !oric->tapebuf )
  {
    // Try appending .tap
    strcpy( &tapefile[i], ".tap" );
    tape_load_tap( oric, tapefile );
  }
  if( !oric->tapebuf )
  {
    // Try appending .ort
    strcpy( &tapefile[i], ".ort" );
    tape_load_tap( oric, tapefile );
  }
  
  if( oric->tapebuf )
  {
    // We already inserted this one. Don't re-insert it when we get to the end. */
    oric->lasttapefile[0] = 0;
  }
    
  chdir( odir );
  free( odir );
}
I think this is because tape_autoinsert line 1088 first try to load "PROG2" file, through tape_load_tap(), file which doesn't exist.
But the oric->tapebuf isn't empty (it contains PROG1.TAP file) so there is no try with .tap or .ort extension.
To correct that you can modify tape_patches in the same paragraph and add

Code: Select all

if ( oric->tapeoffs >= oric->tapelen -1) tape_eject( oric );
right before the

Code: Select all

tape_autoinsert( oric );
or you can modify tape_autoinsert to check the return code of tape_load_tap
something like:

Code: Select all

void tape_autoinsert( struct machine *oric )
{ 
  char *odir;
  int i;
  SDL_bool tape_found;

  if( strncmp( (char *)&oric->mem[oric->pch_fd_getname_addr], oric->lasttapefile, 16 ) == 0 )
    oric->mem[oric->pch_fd_getname_addr] = 0;

  // Try and load the tape image
  strcpy( tapefile, oric->lasttapefile );
  i = (int)strlen(tapefile);

  odir = getcwd( NULL, 0 );
  chdir( tapepath );
  tape_found = tape_load_tap( oric, tapefile );
  if( !tape_found )
  {
    // Try appending .tap
    strcpy( &tapefile[i], ".tap" );
    tape_found = tape_load_tap( oric, tapefile );
  }
  if( !tape_found )
  {
    // Try appending .ort
    strcpy( &tapefile[i], ".ort" );
    tape_load_tap( oric, tapefile );
  }
  
  if( oric->tapebuf )
  {
    // We already inserted this one. Don't re-insert it when we get to the end. */
    oric->lasttapefile[0] = 0;
  }
    
  chdir( odir );
  free( odir );
}
I'm not sure if the last test if( oric->tapebuf ) must be modify or not.

The whole patch is (r644 and above):

Code: Select all

--- tape.c	2014-11-16 22:53:00.000000000 +0100
+++ tape.c.new	2015-07-14 15:07:14.104774319 +0200
@@ -1075,6 +1075,7 @@
 {
   char *odir;
   int i;
+  SDL_bool tape_found;
 
   if( strncmp( (char *)&oric->mem[oric->pch_fd_getname_addr], oric->lasttapefile, 16 ) == 0 )
     oric->mem[oric->pch_fd_getname_addr] = 0;
@@ -1085,14 +1086,14 @@
 
   odir = getcwd( NULL, 0 );
   chdir( tapepath );
-  tape_load_tap( oric, tapefile );
-  if( !oric->tapebuf )
+  tape_found = tape_load_tap( oric, tapefile );
+  if( !tape_found )
   {
     // Try appending .tap
     strcpy( &tapefile[i], ".tap" );
     tape_load_tap( oric, tapefile );
   }
-  if( !oric->tapebuf )
+  if( !tape_found )
   {
     // Try appending .ort
     strcpy( &tapefile[i], ".ort" );
@@ -1161,7 +1162,7 @@
           // Only do this if there is no tape inserted, or we're at the
           // end of the current tape, or the filename ends in .TAP, .ORT or .WAV
           if( ( !oric->tapebuf ) ||
-              ( oric->tapeoffs >= oric->tapelen ) ||
+              ( oric->tapeoffs >= oric->tapelen -1 ) ||
               ( ( i > 3 ) && ( strcasecmp( &oric->lasttapefile[i-4], ".tap" ) == 0 ) ) ||
               ( ( i > 3 ) && ( strcasecmp( &oric->lasttapefile[i-4], ".ort" ) == 0 ) ) ||
               ( ( i > 3 ) && ( strcasecmp( &oric->lasttapefile[i-4], ".wav" ) == 0 ) ) )
Maybe someone else can check this patch.

Re: Oricutron: BUG with .tap files

Posted: Mon Jul 20, 2015 9:45 am
by Hialmar
I have put this in revision r665.

I have corrected one little thing:

Code: Select all

  tape_found = tape_load_tap( oric, tapefile );
  if( !tape_found )
  {
    // Try appending .tap
    strcpy( &tapefile[i], ".tap" );
    tape_found = tape_load_tap( oric, tapefile ); // added tape_found = so that the next test is okay
  }
  if( !tape_found )
  {
    // Try appending .ort
    strcpy( &tapefile[i], ".ort" );
    tape_load_tap( oric, tapefile );
  }

Re: Oricutron: BUG with .tap files

Posted: Mon Jul 20, 2015 11:47 am
by iss
I checked the update and regardless its number (r666) it works ;).

Re: Oricutron: BUG with .tap files

Posted: Mon Jul 20, 2015 2:02 pm
by Hialmar
Hehe the r666 is only me reverting to a standard Oricutron.cfg.
Let's hope this won't bring the antechrist to earth ;)