Valgrind errors

Hello I am fairly new to coding in c++ and I had some question about the errors I have been getting on my code. So I have this code where I have to read in some words from a text file, where the user can choose which file they want to work with, into an array using a buffer, and after that the code chooses a random word(random number), scrambles the word, and the user has to guess. The code works but I am having some valgrind errors and I am not sure how to resolve them, Any ideas?

// wscramble.cpp
// Word Scramble guessing game
// Illustrates string library functions, character arrays,
// arrays of pointers, etc.
#include <iostream>
#include <cstdlib>
#include <ctime>
#include <cstring>
//#1 include the right header
#include <fstream>
#include <string>
#define SIZE 40
using namespace std;

// Scramble the letters of this string randomly
void permute(char items[], int len);


//#2 deletes global word bank and numwords declaration
/* Delete the array below and replace by reading in words from a file
in main() */
/* const char* wordBank[] = {
"computer", "president", "trojan", "program", "coffee",
"library", "football", "popcorn", "science", "engineer"}; */
//const int numWords = 10;

int main(int argc, char *argv[]) {
srand(time(0));
char guess[80];

bool wordGuessed = false;
int numTurns = 10;

//#3 commandline check
//explanation for self: the point of this is to get whether the user wants
//to work with the english or spanish file
string choosefile;
getline(cin, choosefile);

//#4 file checks
//explanation for self: see's if can open file
ifstream file;
file.open(choosefile.c_str());
if(file.fail())
cout << "file open fail." << endl;
//#5 word count input
//explanation for self: attempts to read word count into file
int wordcount;
file >> wordcount;
if(file.fail()){
file.close();
cout << "word count input fail." << endl;
}

/* Add code to declare the wordBank array, read in each word
and store it */
//#6 dynamincally allocate an array
char** wordBank = new char*[wordcount + 1];
//#7 create buffer
char buffer[41]; //shallowcopy an array that points to what it is, this consistently changes
//#8 reading in words into allocated array
if(file.is_open()){
int i = 0;
while(!file.eof()){
//how to read in words from file into wordBank array?
//each line is a different array element
file >> buffer;
int eachwordlen = strlen(buffer);
char* deepcopy = new char[eachwordlen + 1];
strcpy(deepcopy, buffer);
wordBank[i] = deepcopy;
i++;
delete [] deepcopy;
}
}

// Pick a random word from the wordBank
int target = rand() % wordcount;
int targetLen = strlen(wordBank[target]);

// Make a dynamically-allocated copy of the word and scramble it
char* word = new char[targetLen+1];
char* scrambledcopy = new char[targetLen+1];
strcpy(word, wordBank[target]);
strcpy(scrambledcopy, word);
permute(scrambledcopy, targetLen);


// An individual game continues until a word
// is guessed correctly or 10 turns have elapsed
while (!wordGuessed && numTurns > 0) {
cout << "Scrambled word: " << scrambledcopy << endl;
cout << "What do you guess the original word is? ";
cin >> guess;
/* Complete the next line of code */
if((string)guess != (string)word){
numTurns--;
continue;
}
else{
wordGuessed = true;// ADD code to check if the guess is equal to the chosen word.
// wordGuessed should be true if they are equal, and false otherwise
numTurns--;
break;
}
// Every guess counts as a turn
}

if (wordGuessed) {
cout << "You win!" << endl;
}
else {
cout << "Too many turns...You lose!" << endl;
}

/* Free up any necessary memory */
file.close();
delete [] word;
delete [] scrambledcopy;
delete [] wordBank;
return 0;
}

// Scramble the letters. See "Knuth shuffle".
void permute(char items[], int len) {
for (int i = len-1; i > 0; --i) {
int r = rand() % i;
char temp = items[i];
items[i] = items[r];
items[r] = temp;
}
}

Valgrind errors:
=276== HEAP SUMMARY:
==276== in use at exit: 0 bytes in 0 blocks
==276== total heap usage: 15 allocs, 15 frees, 83,614 bytes allocated
==276==
==276== All heap blocks were freed -- no leaks are possible
==276==
==276== For counts of detected and suppressed errors, rerun with: -v
==276== ERROR SUMMARY: 16 errors from 4 contexts (suppressed: 0 from 0)
codio@polkastock-numericopinion:~/workspace$ g++ -Wall -g scramble.cpp -o scramble
codio@polkastock-numericopinion:~/workspace$ valgrind --tool=memcheck --leak-check=yes ./scramble wordbank.txt
==282== Memcheck, a memory error detector
==282== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==282== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==282== Command: ./scramble wordbank.txt
==282==
wordbank.txt
==282== Invalid read of size 1
==282== at 0x4C32CF2: strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==282== by 0x109816: main (scramble.cpp:79)
==282== Address 0x5b80440 is 0 bytes inside a block of size 7 free'd
==282== at 0x4C3173B: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==282== by 0x1097D8: main (scramble.cpp:73)
==282== Block was alloc'd at
==282== at 0x4C3089F: operator new[](unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==282== by 0x109770: main (scramble.cpp:69)
==282==
==282== Invalid read of size 1
==282== at 0x4C32D04: strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==282== by 0x109816: main (scramble.cpp:79)
==282== Address 0x5b80441 is 1 bytes inside a block of size 7 free'd
==282== at 0x4C3173B: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==282== by 0x1097D8: main (scramble.cpp:73)
==282== Block was alloc'd at
==282== at 0x4C3089F: operator new[](unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==282== by 0x109770: main (scramble.cpp:69)
==282==
==282== Invalid read of size 1
==282== at 0x5558BC0: __strcpy_ssse3 (strcpy-ssse3.S:32)
==282== by 0x10987F: main (scramble.cpp:84)
==282== Address 0x5b80440 is 0 bytes inside a block of size 7 free'd
==282== at 0x4C3173B: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==282== by 0x1097D8: main (scramble.cpp:73)
==282== Block was alloc'd at
==282== at 0x4C3089F: operator new[](unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==282== by 0x109770: main (scramble.cpp:69)
==282==
==282== Invalid read of size 1
==282== at 0x4C32E03: strcpy (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==282== by 0x10987F: main (scramble.cpp:84)
==282== Address 0x5b80441 is 1 bytes inside a block of size 7 free'd
==282== at 0x4C3173B: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==282== by 0x1097D8: main (scramble.cpp:73)
==282== Block was alloc'd at
==282== at 0x4C3089F: operator new[](unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==282== by 0x109770: main (scramble.cpp:69)
==282==

Last edited on
Please edit your question and put "code tags" around your code. They look like this:
[code]
[/code]


If you are allowed to use C++ std::string instead of c-style strings then you should do so.
Your basic problem is that you are assigning the same buffer address to every array element, so they all end up giving you the last word that was read in (the current value of buffer).

Also, your "permute" algorithm is a little off.
Last edited on
As C++, then consider:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
#include <iostream>
#include <fstream>
#include <string>
#include <random>
#include <vector>

std::mt19937 rng(std::random_device {}());

int main() {
	std::string choosefile;

	std::cout << "Enter file name: ";
	std::getline(std::cin, choosefile);

	std::ifstream file(choosefile);

	if (!file)
		return (std::cout << "file open fail.\n"), 1;

	unsigned wordcount {};

	if (!file >> wordcount)
		return (std::cout << "word count input fail.\n"), 2;

	std::vector<std::string> wordBank;

	for (std::string word; file >> word; wordBank.push_back(word));

	//for (const auto& w : wordBank)
		//std::cout << w << '\n';

	std::uniform_int_distribution<unsigned> distrib(0, wordBank.size() - 1);
	std::string& word {wordBank[distrib(rng)]};
	std::string shuWrd {word};
	std::string guess;
	bool wordGuessed {};

	std::shuffle(shuWrd.begin(), shuWrd.end(), rng);

	for (unsigned numTurns {10}; !wordGuessed && numTurns > 0; --numTurns) {
		std::cout << "You have " << numTurns << " guesses remaining\n";
		std::cout << "Scrambled word: " << shuWrd << '\n';
		std::cout << "What do you guess the original word is? ";
		std::cin >> guess;
		if (!(wordGuessed = (guess == word)))
			std::cout << "Incorrect guess\n";
	}

	if (wordGuessed)
		std::cout << "You win!\n";
	else
		std::cout << "Too many turns...You lose!\n";
}


Use std::string where possible. Also use C++ random rather than C rand(). shuffle() will randomly shuffle the data contents. There's no need to have the first entry of the file the number of words to read. This can be determined. So it's read and then discarded. vector is the C++ way to have run-time specified array.
That original code has some dangerous casts to std::string, reads into unbounded buffers, unbounded string copies, ... much of the stuff we hated about old C programs.
1
2
3
4
5
6
==282== Address 0x5b80440 is 0 bytes inside a block of size 7 free'd
==282== at 0x4C3173B: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==282== by 0x1097D8: main (scramble.cpp:73)
==282== Block was alloc'd at
==282== at 0x4C3089F: operator new[](unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==282== by 0x109770: main (scramble.cpp:69)

Well you look at line 69 and line 73 and work out what you did wrong between the two.

Edit your post with code tags.
Topic archived. No new replies allowed.