[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    KTar + KTempFile + QFilePrivate::openExternalFile
From:       Sascha Cunz <sascha.cunz () tiscali ! de>
Date:       2005-09-17 0:32:02
Message-ID: 200509170232.08108.sascha.cunz () tiscali ! de
[Download RAW message or body]

Hi Roberto,
hi kde-core-devels,

i do now know, why KTar does not work.

This is because of KTempFile::file() returns a pointer to a QFile for which 
QFile::fileName() will return an empty string.

This in turn is due to this code:

QFile* KTempFile::file()
{
   [...]
   mFile = new QFile();
   mFile->setFileName( name() );
   mFile->open(QIODevice::ReadWrite, mStream);
   return mFile;
}

The problem is: setFileName will create a QBufferedFSFileEngine. In this fe it 
will set d_ptr->fileName to what KTempFile::name() has returned. The 
following QFile::open(OpenMode, FILE*) call will then delete the 
QBufferedFSFileEngine again. And create a new one. Which has no name.
We cannot switch both lines, because the second QBufferedFSFileEngine will be 
marked as "isOpen" and setFileName on the QFile will fail.

KTar then furhter assumes, that the retuned QFile has to be closed, which is 
not posible according to Qt3.3 and Qt4.0 docs. However, KTar calls 
QFile::close, then later KTar::KTarPrivate::fillTempFile where the QFile is 
closed again (why ever?). No it opens the file for write-only. This is where 
Qt barks, because the QFile has no fileName() attached to it.

Thus, opening a gziped tar with KTar is broken. Further KTempFile::file() is 
broken, because the returned QFile* has no fileName set and cannot be closed 
and reopened.

However, i think, this can be fixed in more than one way:

1. QFile should keep the name that is set for it, even if opening an
   "external" file. (The attached patch against qt-copy does that)
   However, QFile::open(OpenMode,FILE*) documentation contains so much
   warnings, i think at least this fact should go into it's docs.

   Roberto, maybe you can forward the patch to whoever at TT wrote the
   QFile code for discussion. Though, i do not think that it is right
   for QFile to still return the name that was set with setFileName()
   after opening an external file.

2. Simply add a comment to the documentation of KTempFile: "Use
   KTempFile::name() if you want to close and open the file again."
   However, this does not fix the broken result of KTempFile::file()
   I think, that i can change KTar, so that it would not rely on
   KTempFile::file(). However, got no clue how often this technique is
   used in KDE at other places. lxr.kde.org is no big help. One can
   guess that searching for "file" yields _much_ results. And searching
   for KTempFile yields about 1600 references.

3. Sanitize KTempFile. The file could be opened using a QFile from the very
   beinning on. Not my personal favourite, because a short look into QFile doc
   shows no way to get a FILE* or an int fd from it. So KTempFile would loose
   that posibilities.

4. I don't know, but: If there is a way to ask glibc for the file's name and
   path if i provide glibc a filedescriptor or a FILE*, then maybe
   QFilePrivate could be changed to query the name of the external file.

What's the right way to go?

Cheers Sascha

["filenamenotset.patch" (text/x-diff)]

Index: src/corelib/io/qfile.cpp
===================================================================
--- src/corelib/io/qfile.cpp	(Revision 459025)
+++ src/corelib/io/qfile.cpp	(Arbeitskopie)
@@ -82,7 +82,7 @@
 QFilePrivate::openExternalFile(int flags, int fd)
 {
     delete fileEngine;
-    QFSFileEngine *fe = new QFSFileEngine;
+    QFSFileEngine *fe = new QFSFileEngine(fileName);
     fileEngine = fe;
     return fe->open(flags, fd);
 }
@@ -91,7 +91,7 @@
 QFilePrivate::openExternalFile(int flags, FILE *fh)
 {
     delete fileEngine;
-    QBufferedFSFileEngine *fe = new QBufferedFSFileEngine;
+    QBufferedFSFileEngine *fe = new QBufferedFSFileEngine(fileName);
     fileEngine = fe;
     return fe->open(flags, fh);
 }


[prev in list] [next in list] [prev in thread] [next in thread]