Thread: Segmentation fault when comparing dynamic string to literal

  1. #1
    Registered User
    Join Date
    May 2011
    Posts
    23

    Segmentation fault when comparing dynamic string to literal

    I dynamically allocated string "line" because I don't know how large it will get to be.
    I can assign data to line just fine, however, when I try to scan for spaces in the string, it causes a segmentation fault.

    Code:
        string *line = new string;
        *line = log.getLastLogged();
    
        int count, pos = 0;
        int npos = line->length();
    
        for(int pos = 0; pos < npos; pos++) {
            if(line[pos] == " ") {                         //Segfault here
                pos++;
                count++;
            }
            else pos++;
            cout << count << " " << pos << endl;
        }
    I've tried every variation of "line[pos]" I can think of, but they all fail. Any ideas?
    Also, can anyone can tell me why I can't do "while(line[pos] != 0)" in place of the "for" loop?

  2. #2
    spurious conceit MK27's Avatar
    Join Date
    Jul 2008
    Location
    segmentation fault
    Posts
    8,300
    This is not comparing a string with a literal:

    Code:
    if(line[pos] == " ") {
    It's comparing a single byte with a string literal, which is not possible. You want to compare with a single char/integer value:

    Code:
    if(line[pos] == ' ') {
    If you were use your compiler with warnings enabled, it would probably tell you that.

    [Edit: actually because "line" is a pointer, you should also be dereferencing, (*line)[pos], see laserlight's and grumpy's comments below. But if you make line a local string instead of a heap pointer, the way we are recommending, you won't have to do that.]


    You do not need to "dynamically allocate" a string because you don't know how big it will be. All C++ strings grow dynamically, whether they are stack or heap variables. What you want is:

    Code:
    string line(log.getLastLogged());
    There is also the "find_first_of" method you could use in your for loop instead of iterating with [], but that probably does not make any significant difference.
    Last edited by MK27; 06-28-2011 at 12:17 PM.
    C programming resources:
    GNU C Function and Macro Index -- glibc reference manual
    The C Book -- nice online learner guide
    Current ISO draft standard
    CCAN -- new CPAN like open source library repository
    3 (different) GNU debugger tutorials: #1 -- #2 -- #3
    cpwiki -- our wiki on sourceforge

  3. #3
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Quote Originally Posted by screennameless
    I dynamically allocated string "line" because I don't know how large it will get to be.
    That is no reason to dynamically allocate a std::string object. Consider:
    Code:
    std::string name;
    std::getline(std::cin, name);
    Quote Originally Posted by screennameless
    I've tried every variation of "line[pos]" I can think of, but they all fail. Any ideas?
    line is a pointer to a string, so line[pos] is a string. However, line is a pointer to exactly one string, so line[pos] with any value of pos other than 0 is wrong. npos depends on the length of the string, so if your string has anything more than a length of 1, your code is wrong.

    In other words, you have confused strings and array of strings. From what I see, all you need is just a std::string object, not a pointer to it, or an array of them.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  4. #4
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    Your code dynamically allocates line as a single string. Accessing line[pos] is therefore only valid if pos is zero.

    If you are attempting to find space characters within *line, then ....
    Code:
        if (line[pos] == " ")
    needs to be
    Code:
        if ((*line)[pos] == ' ')
    Note that the right hand side is bounded by single quotes (looking for a single space).

    You also do not need to dynamically allocate a new string (i.e. line can be of type string, not a pointer, although you then need to fix subsequent code accordingly). The string object itself (whether the object is created dynamically - with operator new - or not) will manage memory internally, which is what you need if you don't know the size of the string contents. If you do insist on dynamically creating the string, you also need to remember to delete it (this is one key difference of C++ from languages like Java).

    I'm also not clear on why you are incementing pos inside the loop body. It basically means you are skipping every second character of the string.
    Right 98% of the time, and don't care about the other 3%.

    If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

  5. #5
    Registered User
    Join Date
    May 2011
    Posts
    23
    Quote Originally Posted by MK27
    You do not need to "dynamically allocate" a string because you don't know how big it will be. All C++ strings grow dynamically, whether they are stack or heap variables.
    I understand that they grow in the stack. But "line" has the potential to be very long. I don't want it to be the cause, or to help cause a stack overflow in the future.

    Quote Originally Posted by MK27
    Code:
    string line(log.getLastLogged());
    Because I'm sticking to my decision to allocate this to the heap, is there a way to do this without causing a memory leak? Perhaps "string *line = new string(log.getLastLogged());"?

    Quote Originally Posted by MK27
    It's comparing a single byte with a string literal, which is not possible. You want to compare with a single char/integer value:
    I tried this. I get a compiler error:
    Quote Originally Posted by Code Blocks
    error: no match for ‘operator==’ in ‘*(line + ((long unsigned int)(((long unsigned int)pos) * 8ul))) == ' '’

  6. #6
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    You are over-complicating things.

    A string object resizes itself as required (depending on operations you perform on it) and does that dynamically (i.e. the memory managed internally by a string is typically on the heap). Dynamically creating the string does not change that and, in fact, only increases your chance of a coding error by confusing yourself (which you have demonstrated) unless you really understand pointers and memory management.

    Although there are circumstances where you need to worry about stack space, this is not really one of them.
    Right 98% of the time, and don't care about the other 3%.

    If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

  7. #7
    Registered User
    Join Date
    May 2011
    Posts
    23
    Quote Originally Posted by grumpy
    I'm also not clear on why you are incementing pos inside the loop body. It basically means you are skipping every second character of the string.
    Ah, thank you for pointing that out. The loop used to be a while() loop.

    Quote Originally Posted by grumpy
    You are over-complicating things.

    A string object resizes itself as required (depending on operations you perform on it) and does that dynamically (i.e. the memory managed internally by a string is typically on the heap). Dynamically creating the string does not change that and, in fact, only increases your chance of a coding error by confusing yourself (which you have demonstrated).

    Although there are circumstances where you need to worry about stack space, this is not really one of them.
    So, you're saying that strings are on the heap most of the time anyway?

  8. #8
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Quote Originally Posted by screennameless
    I understand that they grow in the stack. But "line" has the potential to be very long. I don't want it to be the cause, or to help cause a stack overflow in the future.
    By default, the contents of std::string are allocated on the heap (or perhaps I should say free store), not the stack.

    Quote Originally Posted by screennameless
    Because I'm sticking to my decision to allocate this to the heap, is there a way to do this without causing a memory leak? Perhaps "string *line = new string(log.getLastLogged());"?
    I would like to help you with this decision, but I disagree vehemently with it. You simply are basing this decision on a serious misconception, and I can do no better than to condemn it wholeheartedly.

    Your misconception is that dynamically allocating a single std::string object somehow allows you to have more space for its contents than just having say a local std::string object. It does not.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  9. #9
    spurious conceit MK27's Avatar
    Join Date
    Jul 2008
    Location
    segmentation fault
    Posts
    8,300
    Quote Originally Posted by screennameless View Post
    I understand that they grow in the stack. But "line" has the potential to be very long. I don't want it to be the cause, or to help cause a stack overflow in the future.
    It cannot cause an overflow, that's how C++ strings work. And as laserlight points out, it's on the heap anyway.

    Because I'm sticking to my decision to allocate this to the heap, is there a way to do this without causing a memory leak?
    My apologies for not noticing the dereference with the use of the assignment operator =, that's not a leak. And for, conversely, not pointing out that line also needs to be dereferenced with the []. I corrected my first post -- maybe you should just ignore me, lol.


    I tried this. I get a compiler error:
    Yeah, that's the dereferencing issue. It should be:

    Code:
    (*line)[pos];
    If you use a pointer, but as has been said, there is no point to that. Make sure you read laserlight and grumpy.
    Last edited by MK27; 06-28-2011 at 12:23 PM.
    C programming resources:
    GNU C Function and Macro Index -- glibc reference manual
    The C Book -- nice online learner guide
    Current ISO draft standard
    CCAN -- new CPAN like open source library repository
    3 (different) GNU debugger tutorials: #1 -- #2 -- #3
    cpwiki -- our wiki on sourceforge

  10. #10
    'Allo, 'Allo, Allo
    Join Date
    Apr 2008
    Posts
    639
    Quote Originally Posted by screennameless View Post
    I understand that they grow in the stack.
    You understand wrong. Three people didn't tell you not to do just to have a laugh when it would (supposedly) backfire, they are actually helping.

  11. #11
    Registered User
    Join Date
    May 2011
    Posts
    23
    Quote Originally Posted by adeyblue View Post
    You understand wrong. Three people didn't tell you not to do just to have a laugh when it would (supposedly) backfire, they are actually helping.
    Alright, alright. Thank you everyone. I won't allocate on the heap. I was simply trying to ensure that I am not, for example, allocating a string on the stack until it gets long, and then having it automatically allocating on the heap. Coming from C, it's difficult to imagine an object doing what a string does without any repercussions.

  12. #12
    Registered User hk_mp5kpdw's Avatar
    Join Date
    Jan 2002
    Location
    Northern Virginia/Washington DC Metropolitan Area
    Posts
    3,817
    Code:
    #include <string>
    
    int main()
    {
        std::string foo("Hello World");
        ...
        return 0;
    }
    The memory for string object foo exists in two places in this example. foo itself is on the stack and only occupies some small fixed sized bit of memory (regardless of how many characters are being managed). The characters themselves are on the heap... it is this part of the string object that grows and shrinks as needed and which the string object manages.

    It works kinda similar to this:
    Code:
    struct my_string
    {
        char * ptr;
    };
    
    int main()
    {
        my_string str;
    
        str.ptr = new char[12];
        strcpy(str.ptr,"Hello World");
        ...
        return 0;
    }
    The my_string variable called str exists on the stack in a manner similar to how the string object foo does. Also, the str variable has memory allocated to it's ptr member from the heap and data copied into this allocated memory similar to how the string object allocates memory and copies the data it manages. In the case of the string object, this allocated data can grow or shrink as needed as operations (concatenation of several strings for example) are made upon the string object.

    So... the string objects themselves are on the stack (unless you create the object itself dynamically) but are fixed and very tiny in the memory they occupy. The data they manage however exists on the heap and can be quite large (millions of characters or larger).
    "Owners of dogs will have noticed that, if you provide them with food and water and shelter and affection, they will think you are god. Whereas owners of cats are compelled to realize that, if you provide them with food and water and shelter and affection, they draw the conclusion that they are gods."
    -Christopher Hitchens

  13. #13
    Officially An Architect brewbuck's Avatar
    Join Date
    Mar 2007
    Location
    Portland, OR
    Posts
    7,396
    Your argument basically goes like this: "I'm declaring the string on the stack, therefore the memory occupied by the string data is on the stack."

    You seem to think this is a general argument, so let's apply it to another situation.

    Code:
    char *buffer = new char[128];
    The pointer buffer is a variable on the stack. Therefore the memory it references must be on the stack. Hopefully you agree that that's incorrect.
    Code:
    //try
    //{
    	if (a) do { f( b); } while(1);
    	else   do { f(!b); } while(1);
    //}

  14. #14
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Quote Originally Posted by screennameless View Post
    Alright, alright. Thank you everyone. I won't allocate on the heap. I was simply trying to ensure that I am not, for example, allocating a string on the stack until it gets long, and then having it automatically allocating on the heap. Coming from C, it's difficult to imagine an object doing what a string does without any repercussions.
    Believe in the standard library designers! They are most likely aware of the stack vs the heap and chose to make a decision about the internal memory management so it won't eat up stack space and crash your application.
    You shouldn't need to worry about memory management at all. What the containers do--and how--is not really for us to know. We just need to know how to use it (the correct way).
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. String assignment segmentation fault (core dump)
    By kapil1089thekin in forum C++ Programming
    Replies: 19
    Last Post: 08-07-2010, 12:51 AM
  2. Segmentation fault when changing a string
    By lilydjwg in forum C Programming
    Replies: 6
    Last Post: 12-02-2009, 07:43 AM
  3. Replies: 7
    Last Post: 07-18-2005, 08:43 AM
  4. segmentation fault when processing a string
    By Nakel in forum C++ Programming
    Replies: 2
    Last Post: 04-24-2003, 04:02 PM
  5. segmentation fault when processing a string
    By EMC2 in forum C++ Programming
    Replies: 0
    Last Post: 04-24-2003, 02:56 PM