[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: [kblocks] /: Fix Coverity #76255: Calling risky function
From: Julian Helfferich <julian.helfferich () googlemail ! com>
Date: 2016-04-30 20:03:05
Message-ID: E1awb6X-0005jj-2W () scm ! kde ! org
[Download RAW message or body]
Git commit 12860515871f2c4e54a55bc9de20ed55a7671507 by Julian Helfferich.
Committed on 30/04/2016 at 20:01.
Pushed by helfferich into branch 'master'.
Fix Coverity #76255: Calling risky function
When loading a plain text replay file, fscanf() was used. The use of
this function is discouraged as it can lead to buffer overflows if the
string read is larger than the reserved buffer.
Now, the C-style functions are replaced with their C++ equivalents.
This means the data is read from an ifstream and stored to a string via
an istringstream.
M +1 -0 KBlocksDefine.cpp
M +1 -0 KBlocksDefine.h
M +61 -34 KBlocksGameReplayer.cpp
M +3 -3 KBlocksGameReplayer.h
M +1 -1 KBlocksRepWin.cpp
http://commits.kde.org/kblocks/12860515871f2c4e54a55bc9de20ed55a7671507
diff --git a/KBlocksDefine.cpp b/KBlocksDefine.cpp
index 06a31f1..1bfcbed 100644
--- a/KBlocksDefine.cpp
+++ b/KBlocksDefine.cpp
@@ -11,6 +11,7 @@
Q_LOGGING_CATEGORY(KBGeneral, "KBlocks.General")
Q_LOGGING_CATEGORY(KBGraphics, "KBlocks.Graphics")
+Q_LOGGING_CATEGORY(KBReplay, "KBlocks.Replay")
Q_LOGGING_CATEGORY(KBSound, "KBlocks.Sound")
const char *KBlocksRecordText[RecordDataType_Max_Count] = {
diff --git a/KBlocksDefine.h b/KBlocksDefine.h
index f204cde..6c70bd2 100644
--- a/KBlocksDefine.h
+++ b/KBlocksDefine.h
@@ -14,6 +14,7 @@
#include <QLoggingCategory>
Q_DECLARE_LOGGING_CATEGORY(KBGeneral)
Q_DECLARE_LOGGING_CATEGORY(KBGraphics)
+Q_DECLARE_LOGGING_CATEGORY(KBReplay)
Q_DECLARE_LOGGING_CATEGORY(KBSound)
#define PREPARE_AREA_WIDTH 5
diff --git a/KBlocksGameReplayer.cpp b/KBlocksGameReplayer.cpp
index bee82b1..6cb51fd 100644
--- a/KBlocksGameReplayer.cpp
+++ b/KBlocksGameReplayer.cpp
@@ -9,27 +9,50 @@
***************************************************************************/
#include "KBlocksGameReplayer.h"
+#include <sstream>
+#include <string>
+
+#include "KBlocksDefine.h"
+
KBlocksGameReplayer::KBlocksGameReplayer(const char *fileName, bool isBinaryMode)
{
+ // Map data types to strings for reading text file
for (int i = 0; i < RecordDataType_Max_Count; ++i) {
mRTMap[ KBlocksRecordText[i] ] = i;
}
mRTMap[string("MaxCount")] = -1;
- FILE *pFile = fopen(fileName, "r");
+ // Set default variables in case loading the file fails
+ mGameCount = 0;
+ mGameSeed = 0;
+ mSameSeed = false;
+ mStepLength = 1;
- if (!pFile) {
- mGameCount = 0;
- mGameSeed = 0;
- mSameSeed = false;
- mStepLength = 1;
+ // Open replay file
+ std::ifstream replayFile;
+ if (isBinaryMode) {
+ replayFile.open(fileName, std::ios::binary);
+ } else {
+ replayFile.open(fileName);
+ }
+
+ // Check that replay file was opened successfully
+ if (!replayFile.is_open()) {
+ qCWarning(KBReplay) << "Unable to open file " << fileName;
return;
}
if (isBinaryMode) {
- loadBinary(pFile);
+ loadBinary(replayFile);
} else {
- loadText(pFile);
+ loadText(replayFile);
+ }
+
+ // Check that more than two Replay steps have been loaded
+ // The two first steps set the required variables.
+ if (mReplayList.size() < 2) {
+ qCWarning(KBReplay) << "Problem loading replay file" << fileName;
+ return;
}
mGameCount = mReplayList.front().value;
@@ -38,9 +61,7 @@ KBlocksGameReplayer::KBlocksGameReplayer(const char *fileName, bool \
isBinaryMode mSameSeed = (mReplayList.front().index == 1);
mReplayList.pop_front();
- mStepLength = 1;
-
- fclose(pFile);
+ replayFile.close();
}
KBlocksGameReplayer::~KBlocksGameReplayer()
@@ -96,48 +117,54 @@ bool \
KBlocksGameReplayer::getNextRecords(vector<KBlocksReplayData> *data) return true;
}
-void KBlocksGameReplayer::loadText(FILE *pFile)
+void KBlocksGameReplayer::loadText(std::ifstream &replayFile)
{
- int count = 0;
- char tmpString[256];
+ std::string line;
+ std::istringstream inStream;
+ std::string tmpString;
KBlocksReplayData tmpData;
mReplayList.clear();
- while (1) {
- count = fscanf(pFile, "%d %s %d %d", &(tmpData.time), tmpString, \
&(tmpData.index), &(tmpData.value));
- tmpData.type = mRTMap[string(tmpString)];
- if ((tmpData.type == -1) || (count != 4)) {
+ do {
+ std::getline(replayFile, line);
+ inStream.str(line);
+ inStream >> tmpData.time >> tmpString >> tmpData.index >> tmpData.value;
+ tmpData.type = mRTMap[tmpString];
+ if ((tmpData.type == -1) || inStream.fail()) {
break;
}
mReplayList.push_back(tmpData);
- }
+ inStream.clear();
+ } while (!replayFile.eof());
}
-void KBlocksGameReplayer::loadBinary(FILE *pFile)
+void KBlocksGameReplayer::loadBinary(std::ifstream &replayFile)
{
KBlocksReplayData tmpData;
mReplayList.clear();
- tmpData.time = fgetc(pFile);
- tmpData.type = fgetc(pFile);
- tmpData.index = fgetc(pFile);
- tmpData.value = fgetc(pFile);
- while (tmpData.time != EOF) {
+
+ tmpData.time = replayFile.get();
+ tmpData.type = replayFile.get();
+ tmpData.index = replayFile.get();
+ tmpData.value = replayFile.get();
+
+ do {
if (tmpData.type == RecordDataType_Skipped) {
int tmpTime = tmpData.time;
while (tmpData.type == RecordDataType_Skipped) {
- tmpData.time = fgetc(pFile);
- tmpData.type = fgetc(pFile);
- tmpData.index = fgetc(pFile);
- tmpData.value = fgetc(pFile);
+ tmpData.time = replayFile.get();
+ tmpData.type = replayFile.get();
+ tmpData.index = replayFile.get();
+ tmpData.value = replayFile.get();
tmpTime += tmpData.time;
}
tmpData.time = tmpTime;
}
mReplayList.push_back(tmpData);
- tmpData.time = fgetc(pFile);
- tmpData.type = fgetc(pFile);
- tmpData.index = fgetc(pFile);
- tmpData.value = fgetc(pFile);
- }
+ tmpData.time = replayFile.get();
+ tmpData.type = replayFile.get();
+ tmpData.index = replayFile.get();
+ tmpData.value = replayFile.get();
+ } while (!replayFile.eof());
}
diff --git a/KBlocksGameReplayer.h b/KBlocksGameReplayer.h
index 2782477..fc83e12 100644
--- a/KBlocksGameReplayer.h
+++ b/KBlocksGameReplayer.h
@@ -12,7 +12,7 @@
#include "KBlocksDefine.h"
-#include <stdio.h>
+#include <fstream>
#include <string>
#include <vector>
#include <list>
@@ -46,8 +46,8 @@ public:
bool getNextRecords(vector<KBlocksReplayData> *data);
private:
- void loadText(FILE *pFile);
- void loadBinary(FILE *pFile);
+ void loadText(std::ifstream &pFile);
+ void loadBinary(std::ifstream &pFile);
private:
int mGameCount;
diff --git a/KBlocksRepWin.cpp b/KBlocksRepWin.cpp
index 5a7f17f..f4da381 100644
--- a/KBlocksRepWin.cpp
+++ b/KBlocksRepWin.cpp
@@ -160,7 +160,7 @@ void KBlocksRepWin::replayOneStep()
{
int tmpPieceChanged = 0;
if (!mpGameLogic->playRecordOneStep(&tmpPieceChanged)) {
- printf("Finished Replay!\n");
+ qCDebug(KBReplay) << "Finished Replay!";
mUpdateTimer.stop();
}
if (tmpPieceChanged != 0) {
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic