-1

I'm writing a function to print a 2D linked-list matrix in row major order, and I'm repeatedly running into a segmentation fault that I can't seem to solve. What is wrong with this code, especially at the col = col->right part? (Edit: Like a few have said, I've added the main part of the code as well, hopefully it provides more context)

struct Node
{   int data;
    struct Node* right;
    struct Node* down;
};

class Matrix{
Node* mhead = NULL;
public:
Node* newNode(int d){
    Node* t = new Node;
    t->data = d;
    t->right = t->down = NULL;
    return t;
} 
void createMatrix(int m, int n){
    Node *node, *mat[n];
    int input;
    for(int i=0; i<m; i++){
        for(int j=0; j<n; j++){
            cin>>input;
            node = newNode(input);
            if(i <= m-1)
                node->down = mat[j];
            mat[j] = node;
            if(j <= n-1)
                mat[j]->right = mat[j+1];
        }
    }
    mhead = mat[0];
}
void printRowCol(){
    
    Node *row = mhead;
    while(row){
        Node *col = row;
        while(col){
            cout<<col->data<<' ';
            col = col->right;
        }
        cout<<"\n";
        row = row->down;
    }
}};

This is the error it produced:

Reading symbols from Solution...done.
[New LWP 868392]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Core was generated by `./Solution'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  Matrix::printRowCol (this=<synthetic pointer>) at Solution.cpp:85
85              cout<<col->data<<' ';
To enable execution of this file add
    add-auto-load-safe-path /usr/local/lib64/libstdc++.so.6.0.25-gdb.py
line to your configuration file "//.gdbinit".
To completely disable this security protection add
    set auto-load safe-path /
line to your configuration file "//.gdbinit".
For more information about this security protection see the
"Auto-loading safe path" section in the GDB manual.  E.g., run from the shell:
    info "(gdb)Auto-loading safe path"
SacredMechanic
  • 133
  • 2
  • 7
  • you should be using `while(row != null)` and `while(col != null)` – Polish Aug 28 '21 at 16:50
  • Maybe your code is fine, but your linked-list matrix isn't formed correctly? Try temporarily changing the `cout` lines to something like `cout << "node\n";` and `cout << "end of row\n";`. Does it finish if you do that, or does it still crash? – jason44107 Aug 28 '21 at 16:55
  • 1
    @Vimal It should be either `while(row != NULL)` or `while(row != nullptr)`. But both are equivalent to `while(row)`. – Lukas-T Aug 28 '21 at 17:00
  • 3
    We have no idea if those pointers are pointing to junk (uninitialized). Testing a pointer for `nullptr` doesn't mean the pointer is valid. What if `mhead` is a garbage value? Please post a [mcve]. – PaulMcKenzie Aug 28 '21 at 17:04
  • 1
    The code seems fine (except for the use of raw pointers). Is it possible the `right` member is not initialized to `nullptr` when `Node` is constructed? Since that's your stopping condition, you need to make sure the end of your linked list (which is what your columns are) is properly terminated. – Bert Aug 28 '21 at 17:06
  • 1
    For issues with pointers, you can't simply pluck out a piece of code where the program crashes. The only reason for the crash is that your pointers are not pointing to valid memory. Since that is the case, then we need to see the program in its entire context to see where, when, and how those pointers are initialized and used. – PaulMcKenzie Aug 28 '21 at 17:10
  • You are using [variable-length arrays](https://en.wikipedia.org/wiki/Variable-length_array), and those [aren't a part of C++](https://stackoverflow.com/questions/1887097/why-arent-variable-length-arrays-part-of-the-c-standard). Use `std::vector` for all your variable-length array needs. – Some programmer dude Aug 28 '21 at 20:56
  • 2
    `mat[j]->right = mat[j+1];` is invalid, as by then you might not have initialized ``mat[j+1]`. Thus `mat[j+1]` contains an indeterminate value, and using such values leads to *undefined behavior*. – Some programmer dude Aug 28 '21 at 20:58
  • 1
    Also, if `j` is equal to `n-1`, then it satisfies the `if` condition, and `j+1` is equal to `n`, and therefore `mat[j+1]` is `mat[n]` which is outside the bounds of `mat`. Buffer overrun. – jason44107 Aug 28 '21 at 20:59
  • `createMatrix()` has numerous errors, making it difficult to pick a place to begin. Focus your debugging efforts on that, use a small test case, step through your code with a debugger, and keep an eye on the values of your variables. – JaMiT Aug 28 '21 at 21:53
  • `*mat[n];` -- This is not valid C++. Arrays in C++ must have their size denoted by a compile-time expression, not a runtime value such as `int n`. Instead `std::vector mat(n);` – PaulMcKenzie Aug 28 '21 at 22:47

1 Answers1

1

It is possible that your code as shown in the question is correct. If the pointers in the data structure were not initialized correctly, the code may crash even when it is correct. For example if the last column had its "right" element left as uninitialized garbage instead of being set to null, during the next iteration there will be an illegal read from a "garbage" memory location.

Gonen I
  • 5,576
  • 1
  • 29
  • 60