→ State changed from open to Revise and resubmit

Your patch looks great!

The only problem I see with the patch is the inclusion of a zip file with unknown origin, as Oswald pointed out. Please update the merge request, perhaps with hand-crafted zip file.

In the worst case we have to include your fix without a testcase…

Can you also elaborate on what “inspired by fallback code from unzip-5.52:zipinfo.c” means?

Simon Hausmann | 5 months ago

In the worst case we have to include your fix without a testcase…
currently, i'm unable to “engineer” a broken zip myself…so, including last commit w/o testcase (or excluding it at all) will be most actual for me at this moment.

“inspired by fallback code from unzip-5.52:zipinfo.c” means “unzip’s fallback code was refactored and adopted to use with QZip in worst case (too old or broken zips)”

ritt.k | 3 months ago

→ State changed from Revise and resubmit to New

gitorious MR-system is broken a bit…again.
this MR shouldn’t contain the last commit (which is selected in MR’s properties).
so, status is “new” once more time…

ritt.k | 2 months ago

→ State changed from New to Reviewing

Not time left to review today but I’ll do that on Monday.

Benjamin Poulain | 24 hours ago

Why do you “Add device() method to QZip* classes”? It does not seems to be used anywhere.

Benjamin Poulain | 16 hours ago

Why do you “Add device() method to QZip* classes”? It does not seems to be used anywhere.

rigth, it isn’t used anywhere in Qt / yet / (isValid and lastModTime aren’t used in Qt as well). but they can be useful for customers (poke me on IRC if you’ll want to hear longer history ;) )

short example:

QZipReader zipReader = new QZipReader(zipfilename, QIODevice::ReadOnly);
if (zipReader->status() != QZipReader::NoError) {
if (QFile f = qobject_cast<QFile>(zipReader->device()))
setError(f->error(), f->errorString()); // inpossible w/o QIODevice device() const method

}

ritt.k | 12 hours ago

→ State changed from Reviewing to Revise and resubmit

rigth, it isn’t used anywhere in Qt / yet / (isValid and lastModTime aren’t
used in Qt as well). but they can be useful for customers (poke me on IRC
if you’ll want to hear longer history ;) )

Fair enough. These classes are private, but the change is small and make sense.

About 3d803c9, could you also change QZipReader::FileInfo::isFile to be false in the default constructor? It does not make sense to have (isFile==true when filePath.isEmpty()==false).

About 28ad6fb, I trust you with the date format.

Benjamin Poulain | 2 hours ago

Thanks :)

This will now go for code scan and then be merged (assuming the scan is ok). The status will remain set to “Reviewing” until this code scan is complete and I've merged it (when I’ll change it to Merged).

thanks

Your patches are finally merged :)
I have pushed the merge on Oslo-staging-1, in the branch of Qt 4.7.