Differential D1511 Diff 10166 source/lib/file/archive/archive_zip.cpp

Changeset View

Standalone View

source/lib/file/archive/archive_zip.cpp

/* Copyright (C) 2017 Wildfire Games.		/* Copyright (C) 2019 Wildfire Games.
		vladislavbelovUnsubmitted Not Done Inline Actions 2019. vladislavbelov: 2019.
*		*
* Permission is hereby granted, free of charge, to any person obtaining		* Permission is hereby granted, free of charge, to any person obtaining
* a copy of this software and associated documentation files (the		* a copy of this software and associated documentation files (the
* "Software"), to deal in the Software without restriction, including		* "Software"), to deal in the Software without restriction, including
* without limitation the rights to use, copy, modify, merge, publish,		* without limitation the rights to use, copy, modify, merge, publish,
* distribute, sublicense, and/or sell copies of the Software, and to		* distribute, sublicense, and/or sell copies of the Software, and to
* permit persons to whom the Software is furnished to do so, subject to		* permit persons to whom the Software is furnished to do so, subject to
* the following conditions:		* the following conditions:
▲ Show 20 Lines • Show All 254 Lines • ▼ Show 20 Lines	public:

void Decompose(size_t& cd_numEntries, off_t& cd_ofs, size_t& cd_size) const		void Decompose(size_t& cd_numEntries, off_t& cd_ofs, size_t& cd_size) const
{		{
cd_numEntries = (size_t)read_le16(&m_cd_numEntries);		cd_numEntries = (size_t)read_le16(&m_cd_numEntries);
cd_ofs = (off_t)read_le32(&m_cd_ofs);		cd_ofs = (off_t)read_le32(&m_cd_ofs);
cd_size = (size_t)read_le32(&m_cd_size);		cd_size = (size_t)read_le32(&m_cd_size);
}		}

		off_t getCommentLength() const
		{
		return static_cast<off_t>(read_le16(&m_comment_len));
		StanUnsubmitted Not Done Inline Actions static_cast<off_t>(expr) Stan: static_cast<off_t>(expr)
		}

private:		private:
u32 m_magic;		u32 m_magic;
u16 m_diskNum;		u16 m_diskNum;
u16 m_cd_diskNum;		u16 m_cd_diskNum;
u16 m_cd_numEntriesOnDisk;		u16 m_cd_numEntriesOnDisk;
u16 m_cd_numEntries;		u16 m_cd_numEntries;
u32 m_cd_size;		u32 m_cd_size;
u32 m_cd_ofs;		u32 m_cd_ofs;
▲ Show 20 Lines • Show All 236 Lines • ▼ Show 20 Lines	static Status ScanForEcdr(const PFile& file, off_t fileSize, u8* buf, size_t maxScanSize, size_t& cd_numEntries, off_t& cd_ofs, size_t& cd_size)
// don't scan more than the entire file		// don't scan more than the entire file
const size_t scanSize = std::min(maxScanSize, size_t(fileSize));		const size_t scanSize = std::min(maxScanSize, size_t(fileSize));

// read desired chunk of file into memory		// read desired chunk of file into memory
const off_t ofs = fileSize - off_t(scanSize);		const off_t ofs = fileSize - off_t(scanSize);
io::Operation op(*file.get(), buf, scanSize, ofs);		io::Operation op(*file.get(), buf, scanSize, ofs);
RETURN_STATUS_IF_ERR(io::Run(op));		RETURN_STATUS_IF_ERR(io::Run(op));

		// Scanning for ECDR first assumes no comment exists
		// (standard case), so ECDR structure exists right at
		// end of file
		off_t offsetInBlock = scanSize - sizeof(ECDR);
		const ECDR* ecdr = nullptr;
		vladislavbelovUnsubmitted Done Inline Actions `nullptr` instead of `NULL`. vladislavbelov: `nullptr` instead of `NULL`.

		for (off_t commentSize = 0; (commentSize <= offsetInBlock) && !ecdr; ++commentSize)
		vladislavbelovUnsubmitted Not Done Inline Actions It looks like a code duplication, we have `FindRecord` function. And you just continue the search, if the `commentSize != ecdr->getCommentLength()` condition is invalid. vladislavbelov: It looks like a code duplication, we have `FindRecord` function. And you just continue the…
		TeiresiasAuthorUnsubmitted Not Done Inline Actions I don't completely understand how the current FindRecord() works: The for-loop runs up pointer p from start to (buf+size-recordSize), testing each position for the magic-id. But the ENSURE(p == start) will fire an assertion if p != start, i.e. the magic-id is not at the very start of the buffer. Regarding "continue the search": I will provide an update for this. Teiresias: I don't completely understand how the current FindRecord() works: The for-loop runs up pointer…
		{
		StanUnsubmitted Not Done Inline Actions Any reason for the new lines ? Stan: Any reason for the new lines ?
		const u8 *pECDRTest = buf + offsetInBlock - commentSize;
		vladislavbelovUnsubmitted Done Inline Actions By CC the brace should be on new line, below too. vladislavbelov: By CC the brace should be on new line, below too.
		StanUnsubmitted Done Inline Actions ++commentSize if you can't use a range for loop. Stan: ++commentSize if you can't use a range for loop.
		if((const u32)pECDRTest == ecdr_magic)
		StanUnsubmitted Not Done Inline Actions static_cast Stan: static_cast
		TeiresiasAuthorUnsubmitted Done Inline Actions static_cast<const u32>(pECDRTest) gives a compiler error "invalid cast from type const u8 (aka unsigned char) to const u32...". reinterpret_cast or classic C-style-cast, as currently used, will pass. Please state your preference. I checked the coding conventions for that and all I found about casting is "Don't use RTTI (dynamic_cast etc). " Is there another CC doc? Teiresias:* static_cast<const u32>(pECDRTest) gives a compiler error "invalid cast from type const u8…
		StanUnsubmitted Not Done Inline Actions We prefer explicit C++ casts, static_cast where possible, reinterpret_cast when necessary, dynamic_cast where there is no choice. So in this case I'd go with reinterpret_cast if that works :) Stan: We prefer explicit C++ casts, static_cast where possible, reinterpret_cast when necessary…
		{
		StanUnsubmitted Not Done Inline Actions Static cast maybe unless you can refactor the thing not to have casts Stan: Static cast maybe unless you can refactor the thing not to have casts
		TeiresiasAuthorUnsubmitted Not Done Inline Actions I tried to go for static_cast<const u32>(), but at least gcc keeps giving me errors "invalid static_cast from type const u8 {aka const unsigned char} to type const u32 {aka const unsigned int} I hesitate from refactoring the thing as this would go out of style of the rest of the file Teiresias:* I tried to go for static_cast<const u32*>(), but at least gcc keeps giving me errors "invalid…
		StanUnsubmitted Not Done Inline Actions Might be nice to figure out why we need to cast to such a smaller size and if everything cannot be changed so that cast is not needed :) Maybe another diff :) Stan: Might be nice to figure out why we need to cast to such a smaller size and if everything cannot…
		// Signature matches, test whether comment
		// fills up the whole space following the
		// ECDR
		ecdr = (const ECDR*)pECDRTest;
		StanUnsubmitted Not Done Inline Actions static_cast or ecdr_magic? Stan: static_cast or ecdr_magic?
		TeiresiasAuthorUnsubmitted Done Inline Actions static_cast leads to a compiler error "invalid cast from const u8* to const ECDF". reinterpret_cast or classic C-style casts work. As for the "or ecdr_magic" alternative, I'm not sure. Do you mean something like "ecdr = &ecdr_magic;"? That does not fit, since ecdr has to grab the position of the whole zip structure within the file content, not only the magic-id. Lateron we do ecdr->Decompose(...) on that. Teiresias:* static_cast leads to a compiler error "invalid cast from const u8* to const ECDF*".
		StanUnsubmitted Not Done Inline Actions Same here for the cast. Oh, I did not notice it was a pointer :/ Just wanted to avoid the cast. Maybe there could be a temporary variable storing the reinterpreted value. Stan: Same here for the cast. Oh, I did not notice it was a pointer :/ Just wanted to avoid the cast.
		if (commentSize != ecdr->getCommentLength())
		{
		// Signature matches but there is some other data between
		vladislavbelovUnsubmitted Done Inline Actions `nullptr` instead of `NULL`. vladislavbelov: `nullptr` instead of `NULL`.
		StanUnsubmitted Done Inline Actions Nullptr :) Stan: Nullptr :)
		// header, comment and EOF. There are three possibilities
		// for this:
		// 1) Header file format and size differ from what we expect
		// 2) File has been truncated
		// 3) The magic id occurs inside a zip comment
		ecdr = nullptr;
		}
		else
		{
		// Seems like a valid archive header before an archive-level
		// comment
		break;
		}
		}
		}

// look for ECDR in buffer		// look for ECDR in buffer
const ECDR* ecdr = (const ECDR*)FindRecord(buf, scanSize, buf, ecdr_magic, sizeof(ECDR));
if(!ecdr)		if(!ecdr)
		StanUnsubmitted Not Done Inline Actions Anyway to avoid the cast here ? :) Stan: Anyway to avoid the cast here ? :)
		TeiresiasAuthorUnsubmitted Not Done Inline Actions I tried to, but found no easy way. UniqueRange.get() returns a "pointer" which is just a void* (see source/lib/allocators/unique_range.h). Maybe one can rewrite this to get rid of the casts, but I don't dare at the moment, since most of the code has proven to work for a long time. Teiresias: I tried to, but found no easy way. UniqueRange.get() returns a "pointer" which is just a void*…
return INFO::CANNOT_HANDLE;		return INFO::CANNOT_HANDLE;

ecdr->Decompose(cd_numEntries, cd_ofs, cd_size);		ecdr->Decompose(cd_numEntries, cd_ofs, cd_size);
return INFO::OK;		return INFO::OK;
}		}

static Status LocateCentralDirectory(const PFile& file, off_t fileSize, off_t& cd_ofs, size_t& cd_numEntries, size_t& cd_size)		static Status LocateCentralDirectory(const PFile& file, off_t fileSize, off_t& cd_ofs, size_t& cd_numEntries, size_t& cd_size)
{		{
const size_t maxScanSize = 66000u; // see below		const size_t maxScanSize = 66000u; // see below
UniqueRange buf(io::Allocate(maxScanSize));		UniqueRange buf(io::Allocate(maxScanSize));

// expected case: ECDR at EOF; no file comment		Status ret = ScanForEcdr(file, fileSize, static_cast<u8*>(buf.get()), maxScanSize, cd_numEntries, cd_ofs, cd_size);
Status ret = ScanForEcdr(file, fileSize, (u8*)buf.get(), sizeof(ECDR), cd_numEntries, cd_ofs, cd_size);
if(ret == INFO::OK)
return INFO::OK;
// worst case: ECDR precedes 64 KiB of file comment
ret = ScanForEcdr(file, fileSize, (u8*)buf.get(), maxScanSize, cd_numEntries, cd_ofs, cd_size);
if(ret == INFO::OK)		if(ret == INFO::OK)
return INFO::OK;		return INFO::OK;

// both ECDR scans failed - this is not a valid Zip file.
io::Operation op(*file.get(), buf.get(), sizeof(LFH));		io::Operation op(*file.get(), buf.get(), sizeof(LFH));
RETURN_STATUS_IF_ERR(io::Run(op));		RETURN_STATUS_IF_ERR(io::Run(op));
// the Zip file has an LFH but lacks an ECDR. this can happen if		// the Zip file has an LFH but lacks an ECDR. this can happen if
// the user hard-exits while an archive is being written.		// the user hard-exits while an archive is being written.
// notes:		// notes:
// - return ERR::CORRUPTED so VFS will not include this file.		// - return ERR::CORRUPTED so VFS will not include this file.
// - we could work around this by scanning all LFHs, but won't bother		// - we could work around this by scanning all LFHs, but won't bother
// because it'd be slow.		// because it'd be slow.
▲ Show 20 Lines • Show All 200 Lines • Show Last 20 Lines