Segmentation fault during counting of elements in array of strings c++

Question

I am trying to solve an old problem found on topcoder. I am immediately stuck in trying to find the number of elements in an array of strings. Here is my code

#include <iostream>
#include <stdio.h>
#include <stdlib.h>
#include <string>

using namespace std;

class MiniPaint {
private:
    size_t numLines;    

public:
    int leastBad(string picture[], int maxStrokes) {
        numLines = 0;
        while (!picture[numLines].empty()) {
            numLines++;
        }

        cout << numLines << '\n';


        return 0;
    }

};


int main() {

    MiniPaint instance;
    string picture[] = {"BBBBBBBBBBBBBBB", "WWWWWWWWWWWWWWW", "WWWWWWWWWWWWWWW", "WWWWWBBBBBWWWWW"};


    instance.leastBad(picture, 10);
    return 0;
}

This code gives me a segmentation fault. Something is going wrong, the code is a little bit excessive for just the functionality of counting the number of elements but of course I want to extend the class to include more functionality. If anyone can explain what is going wrong here I would be grateful! Thanks in advance.

EDIT: when I expand the code by

 cout << picture[numlines] << '\n';

in the while loop, to show the actual elements in the array, first the four proper strings are shown and then somehow it endlessly prints spaces to the terminal. So the problem lies somewhere in the fact that

picture[4].empty()

does not return true, even though picture has only four elements.

There's no `empty()` string to inidcate the end of your array actually. — πάντα ῥεῖ, Sep 01 '16 at 10:35
picture[4] returns 5th element, which is missing from the array. In C/C++ array index begins with 0. You are accessing unknown memory and the program is assuming it is a string and calling it's empty() function, which in reality is probably not, this causes segmentation fault. — Anže, Sep 01 '16 at 10:42
Only arrays created from string literals are automatically given a termination indicator. — molbdnilo, Sep 01 '16 at 10:43

score 5 · Accepted Answer · answered Sep 01 '16 at 10:35

5

Your while loop condition assumes that the last string in the array is empty:

int leastBad(string picture[], int maxStrokes) {
    numLines = 0;
    while (!picture[numLines].empty()) {

But your input string array defined in main() is not terminated with an empty "" string.

So you may want to add this empty string terminator:

// inside main()

string picture[] = {..., "" /* Empty string terminator */ };

In addition, in modern C++ I'd encourage you to use array container classes instead of raw C-style arrays, typically std::vector<std::string>.

In this case, you can use the size() method to get the array size (i.e. element count), or just a range-for loop for iterating through the whole array.

answered Sep 01 '16 at 10:35

Mr.C64

41,637
14
86
162

Thanks, I would indeed prefer to use the container classes like you mention. However, this is for a topcoder problem I am trying to solve, where the input type is not up to me. See https://community.topcoder.com/stat?c=problem_statement&pm=1996&rd=4710 – Slugger Sep 01 '16 at 10:40
@Slugger The problem requirements are formulated for Java. If you want to solve it in C++ you have to adjust the requirements accordingly. – molbdnilo Sep 01 '16 at 10:46
Ah sorry that was my bad then. Thanks for the help, still the answers gave me more insight into why what I wanted to do is not possible using a pointed to an array! – Slugger Sep 01 '16 at 10:55

Nikita · Answer 2 · 2016-09-01T10:56:12.253

0

You are out of array bounds at picture[numLines]. You should pass array length or calculate it and check the index numLines. Code will look like:

size_t length = sizeof(picture) / sizeof(*picture); // For VS use _countof macro
while (numLines < length && !picture[numLines].empty())
{
  ++numLines;
}

edited Sep 01 '16 at 10:56

answered Sep 01 '16 at 10:35

Nikita

6,270
2
24
37

Thanks for the comment, but this would require that I know the number of elements in advance right? – Slugger Sep 01 '16 at 10:38
@Slugger The main thing is to check the length. Length can be calculated. Check this question - ["How do I find the length of an array"](http://stackoverflow.com/questions/4108313/how-do-i-find-the-length-of-an-array) – Nikita Sep 01 '16 at 10:39

score 0 · Answer 3 · answered Sep 01 '16 at 10:35

0

You access the array out of bounds.

When you call picture[4] you want to access a string object which is not there end the call to the function empty() is on uninitialized memory.

You either need to store how big the array is and iterate until numLines<=3 or you can use a vector

std::vector<std::string> picture = ...

for(std::string line : picture)
{
    //do stuff
}

answered Sep 01 '16 at 10:35

Hayt

5,210
30
37

Thanks for the comment, but this would require that I know the number of elements in advance right? – Slugger Sep 01 '16 at 10:38
Yes. That is the drawback with native arrays. vectors are there to avoid this problem. They have an internal array and count for you how many elements there are – Hayt Sep 01 '16 at 10:39

Segmentation fault during counting of elements in array of strings c++

3 Answers3