Title: [287299] trunk/Source/WebCore
Revision
287299
Author
hironori.fu...@sony.com
Date
2021-12-20 23:19:39 -0800 (Mon, 20 Dec 2021)

Log Message

[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*.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (287298 => 287299)


--- trunk/Source/WebCore/ChangeLog	2021-12-21 06:22:38 UTC (rev 287298)
+++ trunk/Source/WebCore/ChangeLog	2021-12-21 07:19:39 UTC (rev 287299)
@@ -1,3 +1,15 @@
+2021-12-20  Fujii Hironori  <hironori.fu...@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  <ashva...@apple.com>
 
         [WebIDL] convertVariadicArguments() should return a FixedVector

Modified: trunk/Source/WebCore/platform/network/win/DownloadBundleWin.cpp (287298 => 287299)


--- trunk/Source/WebCore/platform/network/win/DownloadBundleWin.cpp	2021-12-21 06:22:38 UTC (rev 287298)
+++ trunk/Source/WebCore/platform/network/win/DownloadBundleWin.cpp	2021-12-21 07:19:39 UTC (rev 287299)
@@ -41,6 +41,11 @@
     return 0xDECAF4EA;
 }
 
+static void fileCloser(FILE* file) 
+{
+    fclose(file);
+};
+
 const String& fileExtension()
 {
     static const NeverDestroyed<String> extension { ".download"_s };
@@ -59,34 +64,30 @@
     }
 
     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)
@@ -98,36 +99,35 @@
 
     // 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;
     }
+    std::unique_ptr<FILE, decltype(&fileCloser)> bundle(bundlePtr, &fileCloser);
 
-    bool result = false;
-
     // 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.
@@ -135,15 +135,15 @@
     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
@@ -150,14 +150,14 @@
     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
@@ -164,15 +164,12 @@
     // 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
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to