0

I'm not sure exactly what is causing this error, as the code was working for a little while, but I must've changed something that messed it up and I've never been able to get it working again.

This is the data in the text file being loaded into the 2d array:

10  8
0   255 255 255 0   0   255 255 255 0
255 0   255 255 0   0   255 255 0   255
255 255 0   255 255 255 255 0   255 255
255 255 255 0   255 255 0   255 255 255
255 255 255 255 0   0   255 255 255 255
255 255 255 255 0   0   255 255 255 255
255 255 255 0   255 255 0   255 255 255
0   0   0   255 255 255 255 0   0   0

10/8 being the length/height of the array. imagecorrupted.txt is the same as above, but it has a 355 instead of a 255 somewhere in the data.

This is the relevant code I've come up with so far:

int** load(string imageFile, int &length, int &height) {
    ifstream file(imageFile);
    if(file.is_open()) {
        file >> length; // Loads 10 into length
        file >> height; // Loads 8 into height
        int** array = new int*[height];
        for(int i = 0; i < height; i++) {
            array[i] = new int[length];
        }
        for(int i = 0; i < height; i++) {
            for(int j = 0; j < length; j++) {
                file >> array[i][j];
                if(array[i][j] > 255 || array[i][j] < 0) {
                    cout << "Image is corrupted." << endl;
                    file.close();
                    return nullptr;
                }
            }
        }
        file.close();
        return array;
    }
    else {
        cout << "Unable to open file." << endl;
        return nullptr;
    }
}

void show(int **image, int length, int height) {
    cout << "The height of the matrix is: " << height << endl;
    cout << "The length of the matrix is: " << length << endl;
    cout << "The matrix is: " << endl;
    for(int i = 0; i < height; i++) {
        for(int j = 0; j < length; j++) {
            cout << " " << image[i][j]; 
        }
        cout << endl;
    }
}

void invert(int **image, int length, int height) {
    for(int i = 0; i < height; i++) {
        for(int j = 0; j < length; j++) {
            if(image[i][j] == 255) {
                image[i][j] = 0;
            }
            else {
                image[i][j] = 255;
            }
        }
        cout << endl;
    }
}

void free(int **image, int &length, int &height) {
    if(image) {
        for(int i = 0; i < height; i++) {
            if(image[i]) {
                delete[] image[i];
            }
        }
        delete[] image;
    }
}

int main() {
    int height = 0;
    int length = 0;
    int** image = 0;
    image = load("../resource/imagecorrupted.txt", length, height);
    image = load("../resource/image.txt", length, height);
    show(image, length, height);
    invert(image, length, height);
    show(image length, height);
    free(image, length, height);
}

Output:

Image is corrupted.
The height of the matrix is: 8
The length of the matrix is: 10
The matrix is: 
 0 255 255 255 0 0 255 255 255 0
 255 0 255 255 0 0 255 255 0 255
 255 255 0 255 255 255 255 0 255 255
 255 255 255 0 255 255 0 255 255 255
 255 255 255 255 0 0 255 255 255 255
 255 255 255 255 0 0 255 255 255 255
 255 255 255 0 255 255 0 255 255 255
 0 0 0 255 255 255 255 0 0 0
// bunch of whitespace
-bash: line x: xxxxx Segemntation fault

I should add that this is an assignment from class, so there are certain things I'm restricted in doing (Ex. needs to be 2d array instead of vectors).

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
Spago
  • 69
  • 1
  • 1
  • 6
  • You potentially have a huge memory leak if you determine the file is corrupted. You failed to `delete []` the memory. – PaulMcKenzie Feb 20 '19 at 19:52
  • 2
    I suggest using `num_rows` and `num_columns` instead of `height` and `length` to avoid confusion. – R Sahu Feb 20 '19 at 19:53
  • 4
    Stop using manual memory management (`new`/`delete`) and raw pointers. Embrace `std::array`, `std::vector`, `std::unique_ptr`, `std::shared_ptr` and friends... Write *modern* C++, not C++98. – Jesper Juhl Feb 20 '19 at 19:56
  • @RSahu I *really* wish I could, as it's been extremely confusing for me as well... However the function prototypes were given to us by our professor with these parameters and I'm not sure if I'd be allowed to change it. – Spago Feb 20 '19 at 19:56
  • Also if `new` throws an exception, how do you intend to rollback the memory you allocated? What if the file is a milion lines, and an allocation failure occurs when reading line 784,512? That is one reason why your method is one of the worst ways to do this, and the reason why classes such as `vector` exists. – PaulMcKenzie Feb 20 '19 at 19:58
  • @Spago, You should be able to. After all, you are implementing them. – R Sahu Feb 20 '19 at 19:59
  • Note: Do what you have to do to get a good grade, but in the real world you would not pass around a pointer, an `int` and an `int`. You would aggregate them into a data structure. Bonus points for applying [RAII](https://stackoverflow.com/questions/2321511/what-is-meant-by-resource-acquisition-is-initialization-raii) and taking the [Rule of Three/Five](https://en.cppreference.com/w/cpp/language/rule_of_three) into account (or reworking the code to take advantage of the Rule of Zero). – user4581301 Feb 20 '19 at 20:03
  • 2
    @Spago [See this method](https://stackoverflow.com/questions/21943621/how-to-create-a-contiguous-2d-array-in-c/21944048#21944048). Two allocations, two deallocations, with rollback. – PaulMcKenzie Feb 20 '19 at 20:04
  • @PaulMcKenzie I apologize for not showing enough of my code, it's been updated with the `free` function showing. Also thank you for all the advice, I have no doubt the methods you've been showing me are far better than what I've been doing here, I just wish I could use them (have yet to learn `vectors` or `templates` in class)! – Spago Feb 20 '19 at 20:21
  • 1
    @Spago If you remove the `template` line and replace `T` with `int` elsewhere from the answer Paul linked to, you'll have your 2D array of `int` that you can work with - but in a safer way. – Ted Lyngmo Feb 20 '19 at 21:00

1 Answers1

5

You've swapped length and height in function show. The outer loop should be height and the inner length.

rsjaffe
  • 5,600
  • 7
  • 27
  • 39
  • Ah yes, thank you @rsjaffe for that. Was wondering why it was never printing out correctly. I've edited my code with that change, among a few other things as well if you can spot any more errors. – Spago Feb 20 '19 at 20:17
  • 1
    You should have only one question per posting. This is how the format of StackOverflow works. If you have another question, ask it separately, and edit your code *in that new question* to display a [mcve] for that issue. – rsjaffe Feb 20 '19 at 20:32
  • 1
    "*I've edited my code with that change, among a few other things as well if you can spot any more errors*" - StackOverflow is not meant for code reviews. That is what [Code Review](https://codereview.stackexchange.com) is meant for instead. – Remy Lebeau Feb 20 '19 at 20:35
  • @rsjaffe I apologize for not being clear. I'm still getting the same segmentation fault error, I just updated my question to show more code, in case what I had shown wasn't enough to show the actual cause of my error. – Spago Feb 20 '19 at 23:20
  • I should add that this was indeed the cause of my segmentation fault. I did the same thing in a `save()` function as well, so my issue is now solved. Thanks again! @rsjaffe – Spago Feb 20 '19 at 23:27