CAPS library v5.1 - bug suspected

3rd Party Software, Tools & Add-ons for KryoFlux
Post Reply
Sugarbox
Posts: 6
Joined: Thu Jul 03, 2014 1:24 pm

CAPS library v5.1 - bug suspected

Post by Sugarbox » Thu Jul 03, 2014 2:31 pm

Hello,

As I explained previously in the wrong forum, I suspected a bug on the library.

Here it is (people who don't stand technical information, please don't read what is following !)

On the source file CapsImage.cpp of the CapsImage folder, line 404, I read :

Code: Select all

PUBYTE trackbuf = pti->trackdata[rev];
int tracklen = allrev ? pti->rawlen : pti->tracksize[rev];
If I understand correctly what's all here, we try to put on the local variable tracklen the max size of the buffer to read.
Which is pti->tracksize[rev] for multiple rev, and meant to be rawlen if "allrev" is different from 0.

But....

Following, we have (line 420) :

Code: Select all

while (bofs < tracklen) {
    // get actual byte from track buffer
    uint8_t bval = trackbuf[bofs];
Which would lead to return out of buffer values in the following case (which I encountered) :
- allrev = 1
- rev = 1
In these case, the index buffer of the trackbuf would be : Index of first rev + bofs (which can be up to rawlen ) > rawlen.

I expected either tracklen to be equal to pti->tracksize[rev], OR trackbuf to be equal to pti->trackdata[0] in case of allrev...

I hope this make sense

This can lead to misunderstand of values (if lucky with read access of the buffer), or even some exceptions.
With the following correction on line 405 :

Code: Select all

int tracklen = pti->tracksize[rev];
It seems to work better.

Thomas

User avatar
IFW
Posts: 2634
Joined: Mon Nov 08, 2010 2:42 pm

Re: CAPS library v5.1 - bug suspected

Post by IFW » Thu Jul 03, 2014 11:24 pm

That fix is incorrect, because it seems to do what you want to achieve but it really does not :)

You must not read a revolution that does not exist - naturally, that would lead to errors.
Could you describe exactly how do you get rev == 1 if allrev is 1 as well? In this case rev should always be 0.

User avatar
IFW
Posts: 2634
Joined: Mon Nov 08, 2010 2:42 pm

Re: CAPS library v5.1 - bug suspected

Post by IFW » Thu Jul 03, 2014 11:38 pm

Would be great if you could post a file and exact details of how to repro the case you encountered.

Sugarbox
Posts: 6
Joined: Thu Jul 03, 2014 1:24 pm

Re: CAPS library v5.1 - bug suspected

Post by Sugarbox » Fri Jul 04, 2014 9:10 am

Well....
allrev = 1 (allrev is used as a boolean) because of

Code: Select all

int allrev = (pti->trackcnt == pti->rawtrackcnt);
rawtrackcnt = 5 because of :

Code: Select all

int maxrev = min(CAPS_MTRS, wh->trkcnt);
	pti->rawtrackcnt = maxrev;
AND wh->trkcnt = 5.

pti->trackcnt is computed as :

Code: Select all

// generate 1 revolution of data as default, 5 for analyser
	int trackcnt = (di.flag & DI_LOCK_ANA) ? 5 : 1;

	// 5 revolutions if track update is not used
	if (!(di.flag & DI_LOCK_UPDATEFD))
		trackcnt = 5;

	// the number of revolutions can't be more than the number of track revolutions decoded
	if (trackcnt > maxrev)
		trackcnt = maxrev;

	// write track info
	pti->trackcnt = trackcnt;
So, trackcnt = 5 because :
- flag does not contain the DI_LOCK_UPDATEFD bit.
- maxrev = 5

To have a reproductible case :

use this little piece of code :

Code: Select all

// TstCAPS.cpp : définit le point d'entrée pour l'application console.
//

#include <Stdint.h>
#include <string.h>
#include "CapsLibAll.h"


int _tmain(int argc, char* argv[])
{
   SDWORD image = CAPSAddImage ();

   if (image  < 0 )
   {
      return -3;
   }
   
   // Convertion from TCHAR to PCHAR
   if ( CAPSLockImage (image , "Strider.raw" ) == imgeOk )
   {
      CapsImageInfo imgInfo;
      memset (&imgInfo, 0 , sizeof (imgInfo));
      if (CAPSGetImageInfo (&imgInfo, image ) == imgeOk)
      {
         // For each head
         int nbSide = imgInfo.maxhead - imgInfo.minhead + 1;
         for (int head = 0; head < nbSide; head++)
         {
            // For each tracks
            for (int track = imgInfo.mincylinder; track <= imgInfo.maxcylinder; track++)
            {
               // Get track info
               CapsTrackInfo capsTrackInfo;

               memset (&capsTrackInfo, 0 , sizeof (capsTrackInfo));
               if ( CAPSLockTrack (&capsTrackInfo, image, track, head, DI_LOCK_TRKBIT/*DI_LOCK_INDEX*/) == imgeOk)
               {
                  int nbRevolutions = capsTrackInfo.trackcnt;
                  if (nbRevolutions > 0)
                  {
                     // For each revolution of this track
                     for (int rev = 0; rev < nbRevolutions; rev++)
                     {
                        // Do something smart here
                     }
                  }
                  CAPSUnlockTrack (image, track, head) ;
               }
            }
         }
      }

      CAPSUnlockImage (image);
      CAPSRemImage (image);
   }
   return 0;
}
- Use the attached "Strider.raw" ct-raw file
- Link with the latest recompiled librarie

And put some breakpoints on the line described in the pervious message.

Last note (if usefull ?) : I use the DI_LOCK_TRKBIT flag bit.
Attachments
Strider.zip
The CT RAW file that causes the problem
(207.89 KiB) Downloaded 57 times

User avatar
IFW
Posts: 2634
Joined: Mon Nov 08, 2010 2:42 pm

Re: CAPS library v5.1 - bug suspected

Post by IFW » Fri Jul 04, 2014 2:49 pm

Thanks, I'll take a look :)

Sugarbox
Posts: 6
Joined: Thu Jul 03, 2014 1:24 pm

Re: CAPS library v5.1 - bug suspected

Post by Sugarbox » Tue Jul 22, 2014 3:41 pm

Hello,

Did you find anything on it ?

Regards,

Thomas

User avatar
IFW
Posts: 2634
Joined: Mon Nov 08, 2010 2:42 pm

Re: CAPS library v5.1 - bug suspected

Post by IFW » Wed Jul 23, 2014 10:31 am

Haven't had time yet as working on the next DTC release.
btw: as a work around, since you want to iterate all revolutions anyway, you can simply ask the library to return a single revolution only by using DI_LOCK_UPDATEFD - which is pretty much what any other software does.

Sugarbox
Posts: 6
Joined: Thu Jul 03, 2014 1:24 pm

Re: CAPS library v5.1 - bug suspected

Post by Sugarbox » Wed Jul 23, 2014 1:30 pm

Thanks a lot !

If I set the revolution befor the lockTrack function, I can even read weak sectors.
Untill better, it will be enough.

User avatar
IFW
Posts: 2634
Joined: Mon Nov 08, 2010 2:42 pm

Re: CAPS library v5.1 - bug suspected

Post by IFW » Wed Jul 23, 2014 4:59 pm

Yes, CAPSGetInfo & CAPSSetRevolution are very useful for iterating the revolutions sampled. Actually your code is the only one that did not use single revolution retrieval, so the problem was well hidden...

Sugarbox
Posts: 6
Joined: Thu Jul 03, 2014 1:24 pm

Re: CAPS library v5.1 - bug suspected

Post by Sugarbox » Wed Jul 23, 2014 5:39 pm

I think it is because of my paradigm : As I already coded the whole FDC, the only data I want to get from these files are the bitstreams.

Post Reply

Who is online

Users browsing this forum: No registered users and 1 guest