Why does program's calling strncat() generate segmentation fault?

Question

I am having trouble trying to figure how why my g++ compiled program seg faults at a strncat() call.

I've been jumping around this site and general Googling and found a number of similar problems but haven't found any solutions that work for me. This is part of a much larger code and there's only so much I can do to redefine variables as the code isn't mine.

All this section of the code is meant to do is read in the last line of a file, remove the relevant data and concatenate to a char*

When I run it, I get a segmentation fault at the line with strncat(RAM,nextchar,1)

char line[256]={"0"};
char nextchar[10]={"0"};
int length=0;
double rac;
double decc;
bool SPACE=false;
char * RAM="RA: ";
char * DECM="DEC: ";
if(AutoColData.good()){
    while(!AutoColData.eof()) AutoColData.getline(line,256);
    for(int i=0;i<strlen(line);i++){
        nextchar[0]=line[i];
        cout<<line[i];
        if(isspace(nextchar[0])&& !SPACE)   SPACE=!SPACE;
        else if(SPACE && !isspace(nextchar[0])){
            SPACE=!SPACE;
            length++;
        }
        if(length==6) {
          cout<<"\n"<<RAM<<nextchar<<"\n";
          strncat(RAM,nextchar,1);
        }
        else if(length==7) strcat(DECM,nextchar);
    }
}

There are some sloppy choices here, I know (the whole "SPACE" thing is messy). But I don't see any reason for this to Seg Fault. It runs fine up until the line with strncat(). The cout works fine, both strings print and have the right data in them but then strncat fails. I've tried using malloc(), strings and nothing seems to work. If anyone could point out what stupid thing I'm doing, it would be very helpful. Thank you.

Well the first thing is that you're attempting to modify string literals, even despite not reserving space for the extra character(s) you add. Any character pointer that points to a literal should be `const char *`. Perhaps you got this warning, or perhaps your warning level is too low, but GCC spits out `warning: deprecated conversion from string constant to 'char*' [-Wwrite-strings]`. — chris, Jun 08 '12 at 23:13
When you fix the first problem you'll have another problem too with the `while(!AutoColData.eof())` loop: http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong — Flexo, Jun 08 '12 at 23:18
If this is truly C++, then this will be massively easier to get right using `std::string`. Otherwise, copy the `"RA: "` into a large enough buffer before attempting to paste something on the end. — Gort the Robot, Jun 08 '12 at 23:18
So I need to give the string more memory when I declare it? Something along the lines of: char* RAC[32]; Then add in the data RAC="RA: ";? — ofwolf, Jun 08 '12 at 23:23

score 7 · Accepted Answer · answered Jun 08 '12 at 23:21

RAM is wrongly declared as a char *, when it really should be a const char *: String literals are read-only, and you are not allowed to write to them. Your strncat call is simply undefined behaviour.

If you want a writable string, you could make a large enough char array: char RAM[100] = "RA: "; (this will be zero-padded at the back), or better even, just use std::string:

std::string RAM = "RA: ";
RAM += nextchar;

Why does program's calling strncat() generate segmentation fault?

1 Answers1