C++ Seg Fault at end of function. Reference line = closing brace

Question

In conclusion: Thanks so much everyone! All the responses posted below were correct. The initial error was me forgetting to leave room for the NULL terminator. Strcpy() is a dangerous function because when I used it, it didn't know when the end of the 'string' was. Therefore, strcpy() grabbed to much data and overwrote the return address.

EDIT: added more code from the program

SOLVED: Honestly, my initial implementation was crap. I don't even know why I wrote swap that way if I wanted to swap out elements of the array. (At the time, each element only had the a char array in it. So I was able to get away with the old implementation). I have re-written it to:

void swap(ArrayElement list[], int index1, int index2) {
     ArrayElement temp;
     temp = list[index1];
     list[index1] = list[index2];
     list[index2] = temp;
}

I'm having problems with a segmentation fault at the end of the following function.

struct ArrayElement {
    char data[SIZE_OF_ELEMENT];
    // Implemented this way so that I can expand to multiple values later on
}

//In main:
ArrayElement* list = new ArrayElement[NUM_OF_ELEMENTS];

void swap(ArrayElement list[], int index1, int index2) {
     char temp[SIZE_OF_ELEMENT];
     strcpy(temp, list[index2].data);
     strcpy(list[index2].data, list[index1].data);
     strcpy(list[index1].data, temp);
}

The error is a segmentation fault at line 45, which is the ending curly brace of the function. This was compiled using g++. I used gbd to try and debug it and everything works correctly until it hits the curly brace.

I can give more code from the program if it is needed. I don't want to post the entire thing because this is for a class.

`std::swap(list[index2].data, list[index1].data)` would this work for you, just out of curiosity. — andre, Oct 25 '12 at 20:44
You should show at least the context where you're calling the function and what `ArrayElement` is ... — πάντα ῥεῖ, Oct 25 '12 at 20:45
@JimBalter - Only if `.data` is an array. If it is a `char*`, your suggestion is buggy. — Robᵩ, Oct 25 '12 at 20:47
I needed to make an array of "strings" should I not have used char arrays for this? — ghn, Oct 25 '12 at 20:57
With a string in C++, one usually means [std::string](http://en.cppreference.com/w/cpp/string/basic_string) — Olaf Dietsche, Oct 25 '12 at 21:03
@user1477320 You should use `std::string` for representing strings in C++, An array of strings should either be represented as `std::vector` (or `std::array` with C++11). — πάντα ῥεῖ, Oct 25 '12 at 21:03

Olaf Dietsche · Accepted Answer · 2012-10-25T21:05:38.747

5

My best guess is, the string at list[index2].data is larger than temp[] and by copying, you overwrote the stack and the return address.

Try inserting a test for the length:

#include <iostream>

...
int n = strlen(list[index2].data);
std::cerr << "len=" << n << ", SIZE_OF_ELEMENT=" << SIZE_OF_ELEMENT << std::endl;

and see, if n (list[index2].data) is larger than SIZE_OF_ELEMENT

edited Oct 25 '12 at 21:05

answered Oct 25 '12 at 20:44

Olaf Dietsche

72,253
8
102
198

You were right. I got len=240 and SIZE_OF_ELEMENT=30. I don't see how this would be. (I've added more code to the original post). This implementation has worked before using my own test data. When I used the file from the instructor this error started occuring. I will rewrite swap to make temp be an ArrayElement object and use '=' to do the swapping... lets see if it works :) – ghn Oct 25 '12 at 21:08
Thank you in helping me solve my problem. If you want to read a conclusion of what was wrong, I added it to the top of the original post. – ghn Oct 25 '12 at 21:26

score 2 · Answer 2 · answered Oct 25 '12 at 20:52

2

strcpy is a hazardous function. If the length of the input string is SIZE_OF_ELEMENT or more, you will write past the end of your temp array. If you must use a fixed size array as the output array in strcpy, you should test that the strcpy will work before using the function.

Even better is to switch from using char arrays to std::string.

answered Oct 25 '12 at 20:52

David Hammen

32,454
9
60
108

Thank you in helping me solve my problem. If you want to read a conclusion of what was wrong, I added it to the top of the original post. – ghn Oct 25 '12 at 21:26

score 1 · Answer 3 · answered Oct 25 '12 at 20:54

1

Is data defined like this char data[SOME_CONSTANT]? If so then are you sure that SIZE_OF_ELEMENT is large enough? You are remembering the NULL terminator too, right?

If in ArrayElement data is defined like this char *data; and is allocated with malloc at a later time then are you sure that index1 has a buffer large enough for the data in index2 and vice versa? Again, you are remembering the NULL terminator too, right?

answered Oct 25 '12 at 20:54

Nik Bougalis

10,495
1
21
37

I did forget the NULL terminator. That was the initial error. Thank you in helping me solve my problem. If you want to read a conclusion of what was wrong, I added it to the top of the original post. – ghn Oct 25 '12 at 21:28

C++ Seg Fault at end of function. Reference line = closing brace

3 Answers3