Segmentation Fault (Core dumped)

[yucecoder]

When I try to sort 1 million values, I get segmentation fault on following code. It works with <= 100 thousand values, so I am probably missing a small detail. Can you help me out?

#include <iostream>
#include <fstream>
#include <string>
#include <math.h>  
#include <stdlib.h>
#include <cstring>
#include <fstream>
#include <stdio.h>
#include <time.h>
#include <vector>
#include <array>
#include <algorithm>

using namespace std;

void heapify(float array[], string city[], float longs[], float lats[], int len, int i) {
    int largest = i;
    int left = 2 * i + 1;
    int right = 2 * i + 2;

    if(left < len && array[left] > array[largest]) { largest = left; }
    if(right < len && array[right] > array[largest]) { largest = right; }

    if(largest != i) {
        swap(array[i], array[largest]);
        swap(city[i], city[largest]);
        swap(longs[i], longs[largest]);
        swap(lats[i], lats[largest]);
        heapify(array, city, longs, lats, len, largest);
    }
}

void buildHeap(float array[], string city[], float longs[], float lats[], int len) {
    for(int i=len/2-1; i>=0; i--) { heapify(array, city, longs, lats, len, i); }
}

void heapSort(float array[], string city[], float longs[], float lats[], int len) {
    buildHeap(array, city, longs, lats, len);

    for(int i=len-1; i>=0; i--) {
        swap(array[0], array[i]);
        swap(city[0], city[i]);
        swap(longs[0], longs[i]);
        swap(lats[0], lats[i]);
        heapify(array, city, longs, lats, i, 0);
    }
}

//To convert given longitude/latitude to radian
float degree2Radian(float angle) {
    return M_PI * angle / 180.0;
}

//Calculate the distance between given 2 loactions
float calculateDistance(float lat1, float lon1, float lat2, float lon2) {
    float dlon, dlat, a, c, d, R;
    R = 6373; //Radius of the Earth as kilometers (3961 as miles)

    lon1 = degree2Radian(lon1);
    lat1 = degree2Radian(lat2);
    lon2 = degree2Radian(lon2);
    lat2 = degree2Radian(lat2);

    dlon = lon2 - lon1;
    dlat = lat2 - lat1;

    a = pow(sin(dlat/2), 2) + cos(lat1) * cos(lat2) * pow(sin(dlon/2), 2);
    c = 2 * atan2(sqrt(a), sqrt(1-a));
    d = R * c;

    return d;
}

int main(int argc, char** argv) {
    
    int numLoc2Sort, numClosest2Find;
    float givenLat, givenLong;

    char *p;
    numLoc2Sort = strtol(argv[1],&p,10);
    numClosest2Find = strtol(argv[2],&p,10);
    givenLat = strtol(argv[3],&p,10);
    givenLong = strtol(argv[4],&p,10);
    
    vector<string> cities;
    vector<float> longVec, latVec;
    
    //Reading Part
    string line;
    ifstream myfile("location.txt");
    if(myfile.is_open()) {
        while (getline(myfile, line)) {
            unsigned pos = line.find("\t"); 
            
            string city = line.substr(0,pos);
            cities.push_back(city);
            line = line.substr(pos+1);
            pos = line.find("\t");
            
            float latitude = atof(line.substr(0,pos).c_str());
            latVec.push_back(latitude);
            line = line.substr(pos+1);
            
            float longitude = atof(line.substr(0,pos).c_str());
            longVec.push_back(longitude);
        }
        myfile.close();
    }
    else { cout << "File could not be opened." << endl; }

    //Creating array that contains distances for each city to given coordinates
    float distArray[cities.size()];
    for(int i=0; i<cities.size(); i++) {
        float dist = calculateDistance(givenLat, givenLong, latVec[i], longVec[i]);
        distArray[i] = dist;
    }

    //Creating arrays for each variable (Could not do it with vectors somehow)
    string cityArray[numLoc2Sort];
    float latArray[numLoc2Sort], longArray[numLoc2Sort];

    for(int i=0; i<numLoc2Sort; i++) {
        cityArray[i] = cities[i];
        latArray[i] = latVec[i];
        longArray[i] = longVec[i];
    }

    //Sorting
    heapSort(distArray, cityArray, longArray, latArray, numLoc2Sort);

    //Creating output file
    ofstream outfile ("output.txt");
    if (outfile) {
        outfile << "Format of the file:\nCity Name || Latitude of City || Longitude of City || City's Distance to Given Coordinates\n" << endl;
        for(int i=0; i<numClosest2Find; i++) {
        outfile << cityArray[i] << " " << latArray[i] << " " << longArray[i] << " " << distArray[i] << endl;
        }
        outfile.close();
    }
    return 0;
}

Here is the link for the input file: https://drive.google.com/file/d/1sIHemct7wEve0j5PyN9RXBCTVmGlUnCw/view?usp=sharing

[Repeater]

What values are the passing in on the command line?

[Repeater]

This is illegal code.
string cityArray[numLoc2Sort];

Your compiler shouldn't allow it and it can cause memory problems. You're not allowed an array on the stack whose size isn't known at compile time, and the stack is of limited size anyway. Do it with vectors.


//Creating arrays for each variable (Could not do it with vectors somehow)
It's not so hard. Like this:
vector<string> cityArray(numLoc2Sort);

[Repeater]

 for(int i=0; i<numClosest2Find; i++) {
        outfile << cityArray[i] << " " << latArray[i] << " " << longArray[i] << " " << distArray[i] << endl;
        }

This makes no sense. The array is of size numLoc2Sort, but you're treating it as if it is of size numClosest2Find.

If this was a vector, you could just ask it what size it is. Use a vector.

[yucecoder]

What values are the passing in on the command line?

This makes no sense. The array is of size numLoc2Sort, but you're treating it as if it is of size numClosest2Find

I take 4 arguments on command line. ./program numLoc2Sort numClosest2Find x y. According to this, my program finds the distance between (x, y) and all other coordinates in input file. Then it sorts numLoc2Sort of them and writes numClosest2Find of them into an output file. Getting distance and sorting them correctly. The only issue is when I try with 1 million or so, it gives segmentation fault. Input file has more than 1 million inputs by the way. So it isn't a problem with input file.

Your compiler shouldn't allow it and it can cause memory problems. You're not allowed an array on the stack whose size isn't known at compile time, and the stack is of limited size anyway. Do it with vectors.

I tried with vectors first, but I couldn't do the swapping thing for a vector that I do at lines 26-28 and 42-44. I tried iter_swap() but it didn't work. It works on my main however it didn't work in my functions. That's why I tried it with arrays and I think you maybe right that this causes the segmentation fault.

[salem c]

> string cityArray[numLoc2Sort];
> float latArray[numLoc2Sort], longArray[numLoc2Sort];
Because 1M of each of these is pretty much guaranteed to blow your stack allocation right out of the water.

Your average desktop with your average toolchain configuration defaults to a stack size of somewhere between 1MB and 8MB. That single line of two large float arrays is 8MB all by itself.

You need to figure out WHY your vector approach didn't work, and fix it.

Not back-peddle into using arrays.

> I tried with vectors first, but I couldn't do the swapping thing for a vector that I do at lines 26-28 and 42-44
> It works on my main however it didn't work in my functions.
Lemme guess, you passed the vectors by value, instead of by reference.

So yeah, you weren't going to get your results back.

[George PlusPlus]

Creating an array that does not have a const size at compile time is illegal C++.

Some compilers have extensions that allow variable length arrays, but it is not standard C++.

#include <iostream>
#include <fstream>
#include <string>
#include <math.h>  
#include <stdlib.h>
#include <cstring>
#include <fstream>
#include <stdio.h>
#include <time.h>
#include <vector>
#include <array>
#include <algorithm> 

If you are going to use C library functions in C++ code, It is recommended to use the C++ library headers. You partially did this with <cstring>.

#include <iostream>
#include <fstream>
#include <string>
#include <cmath>  
#include <cstdlib>
#include <cstring>
#include <fstream>
#include <cstdio>
#include <ctime>
#include <vector>
#include <array>
#include <algorithm>