Skip to content

Commit

Permalink
[Win] MSVC reports "DownloadBundleWin.cpp(87): error C2362: initializ…
Browse files Browse the repository at this point in the history
…ation of 'magic' is skipped by 'goto exit'" with /std:c++20

https://bugs.webkit.org/show_bug.cgi?id=234504

Reviewed by Alex Christensen.

* platform/network/win/DownloadBundleWin.cpp:
(WebCore::DownloadBundle::appendResumeData):
(WebCore::DownloadBundle::extractResumeData):
Removed goto statements. Use std::unique_ptr for FILE*.



Canonical link: https://commits.webkit.org/245451@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@287299 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
fujii committed Dec 21, 2021
1 parent 0928878 commit 68b96f3
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 42 deletions.
12 changes: 12 additions & 0 deletions Source/WebCore/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
2021-12-20 Fujii Hironori <Hironori.Fujii@sony.com>

[Win] MSVC reports "DownloadBundleWin.cpp(87): error C2362: initialization of 'magic' is skipped by 'goto exit'" with /std:c++20
https://bugs.webkit.org/show_bug.cgi?id=234504

Reviewed by Alex Christensen.

* platform/network/win/DownloadBundleWin.cpp:
(WebCore::DownloadBundle::appendResumeData):
(WebCore::DownloadBundle::extractResumeData):
Removed goto statements. Use std::unique_ptr for FILE*.

2021-12-20 Alexey Shvayka <ashvayka@apple.com>

[WebIDL] convertVariadicArguments() should return a FixedVector
Expand Down
81 changes: 39 additions & 42 deletions Source/WebCore/platform/network/win/DownloadBundleWin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ static uint32_t magicNumber()
return 0xDECAF4EA;
}

static void fileCloser(FILE* file)
{
fclose(file);
};

const String& fileExtension()
{
static const NeverDestroyed<String> extension { ".download"_s };
Expand All @@ -59,34 +64,30 @@ bool appendResumeData(const uint8_t* resumeBytes, uint32_t resumeLength, const S
}

auto nullifiedPath = bundlePath.wideCharacters();
FILE* bundle = 0;
if (_wfopen_s(&bundle, nullifiedPath.data(), TEXT("ab")) || !bundle) {
FILE* bundlePtr = 0;
if (_wfopen_s(&bundlePtr, nullifiedPath.data(), TEXT("ab")) || !bundlePtr) {
LOG_ERROR("Failed to open file %s to append resume data", bundlePath.ascii().data());
return false;
}
std::unique_ptr<FILE, decltype(&fileCloser)> bundle(bundlePtr, &fileCloser);

bool result = false;

if (fwrite(resumeBytes, 1, resumeLength, bundle) != resumeLength) {
if (fwrite(resumeBytes, 1, resumeLength, bundle.get()) != resumeLength) {
LOG_ERROR("Failed to write resume data to the bundle - errno(%i)", errno);
goto exit;
return false;
}

if (fwrite(&resumeLength, 4, 1, bundle) != 1) {
if (fwrite(&resumeLength, 4, 1, bundle.get()) != 1) {
LOG_ERROR("Failed to write footer length to the bundle - errno(%i)", errno);
goto exit;
return false;
}

const uint32_t magic = magicNumber();
if (fwrite(&magic, 4, 1, bundle) != 1) {
if (fwrite(&magic, 4, 1, bundle.get()) != 1) {
LOG_ERROR("Failed to write footer magic number to the bundle - errno(%i)", errno);
goto exit;
return false;
}

result = true;
exit:
fclose(bundle);
return result;
return true;
}

bool extractResumeData(const String& bundlePath, Vector<uint8_t>& resumeData)
Expand All @@ -98,81 +99,77 @@ bool extractResumeData(const String& bundlePath, Vector<uint8_t>& resumeData)

// Open a handle to the bundle file
auto nullifiedPath = bundlePath.wideCharacters();
FILE* bundle = 0;
if (_wfopen_s(&bundle, nullifiedPath.data(), TEXT("r+b")) || !bundle) {
FILE* bundlePtr = 0;
if (_wfopen_s(&bundlePtr, nullifiedPath.data(), TEXT("r+b")) || !bundlePtr) {
LOG_ERROR("Failed to open file %s to get resume data", bundlePath.ascii().data());
return false;
}

bool result = false;
std::unique_ptr<FILE, decltype(&fileCloser)> bundle(bundlePtr, &fileCloser);

// Stat the file to get its size
struct _stat64 fileStat;
if (_fstat64(_fileno(bundle), &fileStat))
goto exit;
if (_fstat64(_fileno(bundle.get()), &fileStat))
return false;

// Check for the bundle magic number at the end of the file
fpos_t footerMagicNumberPosition = fileStat.st_size - 4;
ASSERT(footerMagicNumberPosition >= 0);
if (footerMagicNumberPosition < 0)
goto exit;
if (fsetpos(bundle, &footerMagicNumberPosition))
goto exit;
return false;
if (fsetpos(bundle.get(), &footerMagicNumberPosition))
return false;

uint32_t footerMagicNumber = 0;
if (fread(&footerMagicNumber, 4, 1, bundle) != 1) {
if (fread(&footerMagicNumber, 4, 1, bundle.get()) != 1) {
LOG_ERROR("Failed to read footer magic number from the bundle - errno(%i)", errno);
goto exit;
return false;
}

if (footerMagicNumber != magicNumber()) {
LOG_ERROR("Footer's magic number does not match 0x%X - errno(%i)", magicNumber(), errno);
goto exit;
return false;
}

// Now we're *reasonably* sure this is a .download bundle we actually wrote.
// Get the length of the resume data
fpos_t footerLengthPosition = fileStat.st_size - 8;
ASSERT(footerLengthPosition >= 0);
if (footerLengthPosition < 0)
goto exit;
return false;

if (fsetpos(bundle, &footerLengthPosition))
goto exit;
if (fsetpos(bundle.get(), &footerLengthPosition))
return false;

uint32_t footerLength = 0;
if (fread(&footerLength, 4, 1, bundle) != 1) {
if (fread(&footerLength, 4, 1, bundle.get()) != 1) {
LOG_ERROR("Failed to read ResumeData length from the bundle - errno(%i)", errno);
goto exit;
return false;
}

// Make sure theres enough bytes to read in for the resume data, and perform the read
fpos_t footerStartPosition = fileStat.st_size - 8 - footerLength;
ASSERT(footerStartPosition >= 0);
if (footerStartPosition < 0)
goto exit;
if (fsetpos(bundle, &footerStartPosition))
goto exit;
return false;
if (fsetpos(bundle.get(), &footerStartPosition))
return false;

resumeData.resize(footerLength);
if (fread(resumeData.data(), 1, footerLength, bundle) != footerLength) {
if (fread(resumeData.data(), 1, footerLength, bundle.get()) != footerLength) {
LOG_ERROR("Failed to read ResumeData from the bundle - errno(%i)", errno);
goto exit;
return false;
}

// CFURLDownload will seek to the appropriate place in the file (before our footer) and start overwriting from there
// However, say we were within a few hundred bytes of the end of a download when it was paused -
// The additional footer extended the length of the file beyond its final length, and there will be junk data leftover
// at the end. Therefore, now that we've retrieved the footer data, we need to truncate it.
if (errno_t resizeError = _chsize_s(_fileno(bundle), footerStartPosition)) {
if (errno_t resizeError = _chsize_s(_fileno(bundle.get()), footerStartPosition)) {
LOG_ERROR("Failed to truncate the resume footer off the end of the file - errno(%i)", resizeError);
goto exit;
return false;
}

result = true;
exit:
fclose(bundle);
return result;
return true;
}

} // namespace DownloadBundle
Expand Down

0 comments on commit 68b96f3

Please sign in to comment.