Segmentation fault in code

[plastic]

Good morning. I'm new to programming and need some help. I'm getting segmentation fault in the if statement with the following code.

  void delete_car(car_data &daytona, int index)
{
    for(int i = 0; i < cars.daytona.size(); i++)
    index = read_integer("Which car would you like to delete?: ");
    if (index > cars.daytona[index].model[index].size());
    {
    write_line("No model exists: ");
    }
    if (index >= 0 and index < cars.daytona[index].model[index].size());
    int pole_position = cars.daytona[index].models.size() -1;
    cars.daytona[index].models = cars.daytona[pole_position].models;
    cars.daytona[index].models.pop_back();
}

[kigar64551]

Segmentation Fault means that you were trying to access an "invalid" memory address, either for reading or writing. Typical example would be an invalid (out-of-bounds) index in an array or std::vector access. Also, be aware that a segmentation fault is the "good" case, where the bad memory access becomes apparent. In the "bad" case, you read some "garbage" data, or overwrite some other unrelated data, without noticing!

When you access an array, always make sure that the index you access is in between 0 (inclusive) and length (exclusive!) of the array. The same goes for std::vector or similar C++ container classes: Be sure that the index is in between 0 (inclusive) and vec.size() (exclusive!). Especially when index comes from user input!

[lastchance]

if (index > cars.daytona[index].model[index].size());
(i) That > should be >=
(ii) That ; at the end of the line shouldn't be there!


if (index >= 0 and index < cars.daytona[index].model[index].size());
That ; at the end of the line presumably shouldn't be there. But goodness knows what lines were supposed to be invoked if the logic were true.

[plastic]

lastchance thanks for the help and kind advice it is very welcome.

I have changed the code but I get the same result a segmentation fault when it hits (index >= cars.daytona[index].model[index].size()) in terminal.

[kigar64551]

You should run the code in a debugger or at least print out the exact value of index before the crash.

Make sure that index is less than cars.daytona.size().

And also make sure that index is less than cars.daytona[index].model.size().

...before cars.daytona[index].model[index].size() is accessed !!!


Note: If you use an unsigned type, preferably size_t, for indices, then the >= 0 check is not needed.

[plastic]

kigar64551 thank you for your help. It compiles if I enter a whole number between >=0 or < but throws segmentation errors for any other numbers if that's what you mean.

[keskiverto]

Your code, from current OP, with some style edits:

void delete_car(car_data &daytona, int index)
{
    for ( int i = 0; i < cars.daytona.size(); i++ ) {
        index = read_integer("Which car would you like to delete?: ");
    } // for ends here

    if ( index > cars.daytona[index].model[index].size() ) {
    } // if ends here

    {
        write_line("No model exists: ");
    }

    if ( index >= 0 and index < cars.daytona[index].model[index].size() ) {
    } // if ends here

    int pole_position = cars.daytona[index].models.size() -1;
    cars.daytona[index].models = cars.daytona[pole_position].models;
    cars.daytona[index].models.pop_back();
}

Frankly, some of that makes no sense at all.

Furthermore, who is cars? The function gets only daytona and index as parameters.
The first thing you do is to overwrite the index. Why take it as parameter at all?
How many elements are in cars.daytona ?
How many elements are in cars.daytona[k].model ?
Why do you assume that each model has as many elements as daytona?
(At end of function you change size of a model, so even if sizes were same before, they can't be after.)

[kigar64551]

What I mean is that the following expression, which appears inside your if statement:

cars.daytona[index].model[index].size())

...is only allowed, if at this point you know for sure that index < cars.daytona.size() and index < cars.daytona[index].model.size(). Otherwise you'll have an out-of-bounds access within your if check!

[plastic]

keskiverto thanks for the reply. 1) cars is a procedure I have to call 2) I have to work on that part overwriting 3) arrays are updated in the program, i have a function to add car details and a procedure to read out the input data. 4) Using push.back because I need to delete a car in the array.

kigar64551 I've cleaned up the code but it still throws segmentation fault at the if statement if(index > than array.

[seeplus]

What is the value of index? How many elements has cars.daytona ? How many elements has cars.daytona.model ?

What is the type of car_data ?

[keskiverto]

cars is a procedure I have to call

void delete_car(car_data &daytona, int index)
{
    for ( int i = 0; i < cars.daytona.size(); i++ ) {

We see cars on line 3. By syntax it is not a function, but a variable.
It was not declared in this function, not as local nor as parameter.
Therefore, the cars looks like a global variable.

The function parameter daytona is never used in the function.

You should post your current code so we can see how you have changed it.

[keskiverto]

Ignoring the bounds check issues, lets play with an example. We start with daytona.size() == 4:

a b c d
e f g
h i j
k l

First we get index from user.
index = read_integer("Which car would you like to delete?: ");
User said: index = 1

Then we get pole:

int pole_position = cars.daytona[index].models.size() -1;

In our example data daytona[1].models.size() == 3
Therefore: pole_position = 2

Now we copy models? cars.daytona[index].models = cars.daytona[pole_position].models;
That is cars.daytona[1].models = cars.daytona[2].models;
Our data is thus now:

a b c d
h i j
h i j
k l

Finally, we do pop: cars.daytona[index].models.pop_back();
The result after function is thus:

a b c d
h i
h i j
k l

I have to ask: Is this really what you vision to happen?

[plastic]

No. I made some serious errors with my code, I'm new to c++ so I'm learning step by step. I was also going off some examples of a similar procedure that don't work for the procedure I want to create. What I want to get out of the procedure is to choose a model out of a list then delete that model, only that model. If there is no model in the vector I want to return write_line("No model exists: "); without errors. That's the basic idea of the procedure I want to create.

[keskiverto]

Can you show definition of type car_data?

[plastic]

keskiverto thank you for your help, but the code is now fixed. I corrected the errors.