From 5eb47bed81de1326843e1e3bad41addcfed08d4e Mon Sep 17 00:00:00 2001 From: Bent Bisballe Nyeng Date: Wed, 6 Mar 2019 17:28:57 +0100 Subject: Fix crash if opening Resource with a directory instead of a file. --- plugingui/resource.cc | 16 ++++++++++++++-- test/resource_test.cc | 23 ++++++++++++++++++++--- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/plugingui/resource.cc b/plugingui/resource.cc index 2435668..432f25d 100644 --- a/plugingui/resource.cc +++ b/plugingui/resource.cc @@ -28,11 +28,13 @@ #include #include +#include // rcgen generated file containing rc_data declaration. #include "resource_data.h" -namespace GUI { +namespace GUI +{ // Internal resources start with a colon. static bool nameIsInternal(const std::string& name) @@ -85,7 +87,17 @@ Resource::Resource(const std::string& name) std::fclose(fp); return; } - size_t filesize = ftell(fp); + + long filesize = std::ftell(fp); + + // Apparently fseek doesn't fail if fp points to a directory that has been + // opened (which doesn't fail either!!) and ftell will then fail by either + // returning -1 or LONG_MAX + if(filesize == -1L || filesize == LONG_MAX) + { + std::fclose(fp); + return; + } // Reserve space in the string for the data. externalData.reserve(filesize); diff --git a/test/resource_test.cc b/test/resource_test.cc index 2e88c45..28a244e 100644 --- a/test/resource_test.cc +++ b/test/resource_test.cc @@ -30,10 +30,12 @@ #include "drumkit_creator.h" -class ResourceTester : public GUI::Resource { +class ResourceTester + : public GUI::Resource +{ public: ResourceTester(const std::string& name) - : Resource(name) + : GUI::Resource(name) {} bool probeIsInternal() @@ -42,13 +44,15 @@ public: } }; -class ResourceTest : public DGUnit +class ResourceTest + : public DGUnit { public: ResourceTest() { DGUNIT_TEST(ResourceTest::externalReadTest); DGUNIT_TEST(ResourceTest::internalReadTest); + DGUNIT_TEST(ResourceTest::failTest); } DrumkitCreator drumkit_creator; @@ -70,6 +74,19 @@ public: DGUNIT_ASSERT(rc.valid()); DGUNIT_ASSERT_EQUAL((size_t)1123, rc.size()); } + + void failTest() + { + { + ResourceTester rc("/tmp/"); + DGUNIT_ASSERT(!rc.valid()); + } + + { + ResourceTester rc("no_such_file"); + DGUNIT_ASSERT(!rc.valid()); + } + } }; // Registers the fixture into the 'registry' -- cgit v1.2.3