Repair Logic Bug in Par2cmdline

[S. Jamison Wilde]

Working on my ExQueues project I have seen a persistent issue with the released version of par2.exe 0.4

Basically, repairs are failing due to the 'file being accessed by another application' except the application is par2.exe.

Looking thru the code it looks like the folowing is happening, though I admit I havent spent an extraordinary amount of time debugging this:

1) In Par2Repairer::Process when a repair is required, the par2 calles Par2Repairer::RenameTargetFiles which renames the corrupt files to things like myfile.R46.1 (and renames any complete but misnamed, irrelvant for this discussion).

2) CreateTargetFiles is called to create these new files (myfile.R46), with NO sharing flags set in Diskfile::Create, and keeps these file handles open.

3) The RS Matrix is computed.  However, for whatever reasons, the inputblocks have the original myfile.R46 filenames in the block structure, NOT the renamed 'myfile.r46.1'. 

4) In ProcessData, some code was added in 0.4 (par2repairer.cpp rev 1.5) to 'Only keep source files open when they are actually being used.'  However, this method uses inputblocks to open files for reading.  The problem is that the file it is trying to open ( the newly created file myfile.r46 which doesnt have any data as far as I know) is already open and locked, which causes par2.exe to fail with an 'The process cannot access the file because it is already in use by another process.' (err 32).

I think this is a bigger problem than just file locking however.  I believe that inputblocks has the wrong file names for reading, and should be reading from the renamed files instead.  This could explain some other issues where files are repairable in QuickPar but not in par2cmdline.

I have not tested the CVS mutli-threaded version for this same error, just the released 0.4 source, as I have no idea if the threaded version is usable or tested.

[S. Jamison Wilde]

I have hacked a version that repairs this error.

1)
In par2repairer.h add:
    map<string,string>  hack_renamed_files;       

to the class definition.

2) In par2repairer.cpp RenameTargetFiles(void) add

hack_renamed_files[sourcefile->TargetFileName()] = targetfile->FileName();

after the rename.

3)In par2repairer.cpp ProcessData(...)

add after the following (line 2163 in the released src file):
// Open the new file
lastopenfile = (*inputblock)->GetDiskFile();

[ADD]
string open_file = lastopenfile->FileName();
                map<string,string>::iterator renamed_file = hack_renamed_files.find(lastopenfile->FileName());

if(renamed_file != hack_renamed_files.end() && renamed_file->second != "")
  open_file = renamed_file->second;

[CHANGE the very next line ( if (!lastopenfile->Open())
to ]

if (!lastopenfile->Open(open_file))

This ensures that inputblocks opens the correct file rather than the newly created (and empty) file.  

[S. Jamison Wilde]

Hmm, not sure that this patch is working as intended.  So for now I am just going with fixing the file sharing issues in DiskFile::Create and DiskFile::Open (adding FILE_SHARE_WRITE and FILE_SHARE_READ).

[S. Jamison Wilde]

So I finally figured out exactly what was causing my problem: case insensitivity.

Because the usenet application I am using names incomplete files in ALL CAPS, par2.exe was incorrectly trying to open the file once as the_file.r01 and a second time as THE_FILE.r01 even though THE_FILE.r01 had been renamed to the_file.r01.1

The soultion is to make the diskfileMap be case insensitive:

bool DiskFileMap::Insert(DiskFile *diskfile)
{
  string filename = diskfile->FileName();
  assert(filename.length() != 0);

#ifdef WIN32
    // slain wilde: needs to be case insensitive on win32!
    transform(filename.begin(), filename.end(), filename.begin(), tolower);
#endif

  pair<map<string,DiskFile*>::const_iterator,bool> location = diskfilemap.insert(pair<string,DiskFile*>(filename, diskfile));

  return location.second;
}

void DiskFileMap::Remove(DiskFile *diskfile)
{
  string filename = diskfile->FileName();
  assert(filename.length() != 0);

#ifdef WIN32
    // slain wilde: needs to be case insensitive on win32!
    transform(filename.begin(), filename.end(), filename.begin(), tolower);
#endif

    diskfilemap.erase(filename);
}

DiskFile* DiskFileMap::Find(string filename) const
{
  assert(filename.length() != 0);

#ifdef WIN32
    // slain wilde: needs to be case insensitive on win32!
    transform(filename.begin(), filename.end(), filename.begin(), tolower);
#endif

  map<string, DiskFile*>::const_iterator f = diskfilemap.find(filename);

  return (f != diskfilemap.end()) ?  f->second : 0;
}