Strncpy Causing Segmentation Fault c++

Question

I have a c++ program which reads a text file and then converts that text file to a string. Then, it converts the string to a character array using strncpy. I have already seen the stackoverflow question on strncpy and taken the necessary precautions in order to avoid the issues it causes when creating the array. Could someone please explain why it still causes a stack error.

#include <iostream>
#include <string.h>
#include <random>
#include <fstream>
#include <istream>
#include <sstream>
#include <stdio.h>

using namespace std;

int main()
{
    //open a stream reader
    ifstream fin;
    //opens the text file in the stream reader
    fin.open("songlyrics.txt");
    //will be used to aggregate all characters in text file
    string song;
    //used as a pointer when reading each character in text file
    char ch;
    //while the end of file is not reached
    while(!fin.eof())
    {
        //get the character from the file and add it to the song string
        fin.get(ch);
        song += ch;
    }
    //close the file
    fin.close();
    //make a character array called lyrics_ with a length of the length of song
    char lyrics_[song.length()];
    //use strncpy to convert song to a char array, lyrics_
    strncpy(lyrics_, song.c_str(), sizeof(lyrics_));
    //avoid the segmentation fault
    lyrics_[sizeof(lyrics_) - 1] = 0;
    cout<<lyrics_;
    return 0;
}

Also worth noting that `eof()` does not return true until after you've read past the end of the file. You should handle the failure of `fin.get`. — sje397, Dec 02 '14 at 04:23
http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong — n. 'pronouns' m., Dec 02 '14 at 04:37

Barry · Accepted Answer · 2014-12-02T04:26:11.387

3

This:

char lyrics_[song.length()];
//           ^^^^^^^^^^^^^
//           not a compile-time constant

Is a variable-length array, and is not standard C++.

Also, you don't need to convert a std::string to a character array. It kind of already is:

char* lyrics = &song[0]; // assuming you don't append to song in the future

If you really want a separate character buffer, you'll have to allocate it dynamically:

char* lyrics = new char[song.length() + 1];
memcpy(lyrics, song.c_str(), song.length() + 1); // this will copy the null terminator
delete [] lyrics; // don't forget this

edited Dec 02 '14 at 04:26

answered Dec 02 '14 at 04:20

Barry

247,587
26
487
819

@yzt - stop making that edit, I made it a `char*` on purpose. – Barry Dec 02 '14 at 04:26
@Barry: The code you have written is wrong. There is no guarantee in the standard that a `std::string` is a *contiguous* block of characters. – yzt Dec 02 '14 at 04:30
@yzt There is in C++11, and compilers already implemented it this way before the new update. – Barry Dec 02 '14 at 04:31
@Barry Oh I got ya. Thanks, but that's really strange because when i compiled the code the first few times I got no segfault and then it suddenly started happening. But now when I run the code for a separate buffer and I output it to the console, I should get the whole array, but I get no output. – Orren Ravid Dec 02 '14 at 04:53
@Barry I also totally didn't understand the difference between the c and c++ usages of the string and char types until now. My code is a lot simpler now that I just used the string as the array instead of trying to create a new array. Like you said, I don't need to convert a string to a character array. Now everything makes a lot more sense thanks. – Orren Ravid Dec 02 '14 at 05:09

score 1 · Answer 2 · answered Dec 02 '14 at 04:36

C++ does not support the variable length array feature from C. VLA was a standard feature of C.1999, and an optional feature in C.2011.

If you want to make a copy of the contents of a string into a dynamically sized array of char, you can use a vector:

std::vector<char> lyrics_(song.begin(), song.end());
lyrics_.push_back('\0');
std::cout << &lyrics_[0];

Strncpy Causing Segmentation Fault c++

2 Answers2