segmentation fault issue # n

Question

Is this command line wrong? FILE *TM = fopen("TM","r");

When I compile my code with: g++ -O3 -march=corei7 -mtune=corei7 -std=c++11 prueba3.cpp -o prueba3 -lstdc++ or g++ -O3 -march=corei7 -mtune=corei7 -std=c++0x prueba3.cpp -o prueba3 -lstdc++ and then run the executable, I got:

Segmentation fault (core dumped)

When I debug the code with dbg I got the following result:

Reading symbols from ./a.out...done.
(gdb) b main
Breakpoint 1 at 0x400e59: file prueba3.cpp, line 10.
(gdb) r
Starting program: /home/alejo/Desktop/CM/a.out 

Breakpoint 1, main () at prueba3.cpp:10
10  {
(gdb) s
19      FILE *TM = fopen("TM","r");
(gdb) c
Continuing.
2053    2618
2618    3223
3223    3431

Program received signal SIGSEGV, Segmentation fault.
0x000000000040138e in main () at prueba3.cpp:98
98          A[b][a]=A[b][a]+1;
(gdb) q
A debugging session is active.

    Inferior 1 [process 3977] will be killed.

Quit anyway? (y or n) y

But, still, I don't understand how to apply it on my code, which is:

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

using namespace std;

int main()
{
int a,b,i,j,div,tm,ler;  
char string0[256],string1[256],string2[256];
/////////// 
//                          Load files:
//                          TM = size of the sqaure matrix
//                          REL = List of numbers
//                          LER = How many numbers are in REL
/////////// 
FILE *TM = fopen("TM","r");
if(TM == NULL)
{  
    printf("Can't open %s\n","TM");
    exit(1);
}
fscanf(TM,"%255s",string2);
tm = std::stoi(string2);
fclose(TM);

FILE *REL = fopen("REL","r");
if(REL == NULL)
{  
    printf("Can't open %s\n","REL");
    exit(1);
}

FILE *LER = fopen("LER","r");
if(LER == NULL)
{  
    printf("Can't open %s\n","LER");
    exit(1);
}
fscanf(LER,"%255s",string1);
ler = std::stoi(string1);
fclose(LER);

div=ler/2;
///////////     
//                          Allocate matrices
///////////     
int **A;
A = (int **)malloc(tm*sizeof(int*));
for(i=0;i<tm;i++)
{
    A[i]=(int*)malloc(tm*sizeof(int));
}

int *B;
B = (int*) malloc(ler*sizeof(int));
///////////     
//                          Set zero all elementes of allocated matrices
///////////     
if( A != NULL )
{
    for(i=0;i<tm;i++)
    {
        for(j=0;j<tm;j++)
        {
            A[i][j]=0;
        }
    }
}

if( B != NULL )
{
    for(i=0;i<ler;i++)
    {
        B[i]=0;
    }
}
/////////// 
//                          Put the LER numbers of REL in B[i] 
//                          with converting number-string to number-int
///////////     
for(i=0;i<ler;i++)
{
    fscanf(REL,"%255s",string0);
    B[i]=std::stoi(string0);
}
fclose(REL);
/////////// 
//                          Reocate numbers of C[i] in A[i][j]
/////////// 
for(i=0;i<div;i+=2)
{
    a=B[i]-1;
    b=B[i+1]-1;
    std::cout<<a<<"\t"<<b<<"\n";
    A[b][a]=A[b][a]+1;
    A[a][b]=A[a][b]+1;
}   
free(B);
/////////// 
//                          Print A[][]
/////////// 
for(i=0;i<tm;i++)
{
    for(j=0;j<tm;j++)
    {
        cout<<A[i][j]; 
    }
    cout<<"\n"; 
}
free(A);
}

What am I doing wrong?

Just in case, the files are:

REL:

2054
2619
2619
3224
3224
3432
194
2619
2619
3224
3224
3432
448

LER

30846

TM

3434

Answer 1

As you've indicated yourself, the size of A (tm) is 3371 x 3371. The file REL.txt contains numbers like 3383 (line 138 in the file) which are larger than the dimensions of A. This makes you reach out of bounds in this part, as indicated by the debugger:

for(i=0;i<div;i+=2)
{
    a=B[i]-1;
    b=B[i+1]-1;
    std::cout<<a<<"\t"<<b<<"\n";
    A[b][a]=A[b][a]+1;           // b or a may be larger than 3371
    A[a][b]=A[a][b]+1;
}

You don't need to verify this manually though, you can implement it:

for(i=0;i<div;i+=2)
{
    a=B[i]-1;
    b=B[i+1]-1;
    std::cout<<a<<"\t"<<b<<"\n";

    if (a >= tm || b >= tm)
    {
        std::cerr << "Out of bounds!" << std::endl;
        continue;
    }

    A[b][a]=A[b][a]+1;           // b or a may be larger than 3371
    A[a][b]=A[a][b]+1;
}

Answer 2

int *B;
B = (int*) malloc(ler*sizeof(int));

...

if( B != NULL )
{
    for(i=0;i<tm;i++)
    {
        B[i]=0;
    }
}

This is a catastrophe if tm > ler.

Also, why do you condition this code in B being valid, but then unconditionally re-assign values to every element of B? Neither of those things makes sense. If you're going to re-assign values to every element, why zero them? And if you're going to access the array without checking its validity later, why check its validity here?