Segmentation fault with structs and char pointers

This is a discussion on Segmentation fault with structs and char pointers within the C++ Programming forums, part of the General Programming Boards category; Hello! I've (quite) recently begun learning C++ and would appreciate some help with this following problem: The code compiles fine ...

  1. #1
    Registered User
    Join Date
    Jan 2004
    Posts
    10

    Question Segmentation fault with structs and char pointers

    Hello!
    I've (quite) recently begun learning C++ and would appreciate some help with this following problem:
    The code compiles fine but when run it gives me a "Segmentation fault".I hav'nt the slightest of where to begin looking for a fix!

    Possible problem areas:
    Code:
    ...
    bool Equal(char*,char*); // Function prototype
    
    int main()
    {
    ...
    Code:
    ...
    bool Equal(char *str1,char *str2) //Is this the problem?
    {
    
     if(strlen1 != strlen2)
            return(false);
     for(int i=-1;++i<=strlen1; )
            if(str1[i] != str2[i])
                    return(false);
    
     return(true);
    }
    Full Source

    My teacher thought it had something to do with the arguments to "bool Equal()". Any ideas as to the reason of the "Segmentation fault"?
    #include "Sig.h"
    #ifndef noob
    #define noob keybone
    #endif

  2. #2
    Registered User hk_mp5kpdw's Avatar
    Join Date
    Jan 2002
    Location
    Northern Virginia/Washington DC Metropolitan Area
    Posts
    3,806
    Well, this is a problem:
    Code:
    bool Equal(char* str1, char* str2)
    {
        if(strlen1 != strlen2)
            return(false);
        for(int i=-1;++i<=strlen1; )
            if(str1[i] != str2[i])
                    return(false);
        return true;
    }
    First off, either strlen1 and strlen2 are defined and calculated somewhere else or not at all; second, str1[-1] and str2[-1] are not valid positions within the two character arrays. What you need is something more along the lines of this:
    Code:
    int len1 = strlen(str1);
    if( len1 != strlen(str2) )
        return false;
    for(int i = 0; i < len1; ++i )
        if( str1[i] != str2[i] )
            return false;
    return true;
    "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

  3. #3
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,893
    The redundancy, the redundancy!
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  4. #4
    Registered User
    Join Date
    Jan 2004
    Posts
    10
    hk_mp5kpdw:
    Those variables are declared and initiated within bool Equal(), I just forgot to enter them.

    As for the for loop (ach!); as I understand it, i is in this case increased before anything is done(because ++ is before the variable name).Or have I totally gone fishing?

    CornedBee:
    Eh?
    Last edited by Keybone; 01-15-2004 at 06:27 AM.
    #include "Sig.h"
    #ifndef noob
    #define noob keybone
    #endif

  5. #5
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,893
    You have two big redundancies in that code:
    1) You have three for-loops instead of the necessary one. One is in your code, one in each of the strlen calls. Get rid of them.
    2) The function itself is of course redundant, because the CRT already provides strcmp.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  6. #6
    Been here, done that.
    Join Date
    May 2003
    Posts
    1,161
    Originally posted by CornedBee
    1) You have three for-loops instead of the necessary one. One is in your code, one in each of the strlen calls. Get rid of them.
    Sound advice, if you tell them how. Since Keybone is obviously a neo, he probably doesn't understand the 20 ways this can be written. And as for his question, hk_mp5kpdw gave sound advice. Let them learn to program first and when their understanding grows, they can optimize.

    Originally posted by CornedBee
    2) The function itself is of course redundant, because the CRT already provides strcmp.
    Can you think of a better way to understand how to compare two buffers than to write your own version of strcmp? Or are you of the opinion that "if it's already been written, you'll never need to know how it works?"
    Definition: Politics -- Latin, from
    poly meaning many and
    tics meaning blood sucking parasites
    -- Tom Smothers

  7. #7
    Hardware Engineer
    Join Date
    Sep 2001
    Posts
    1,398
    Or have I totally gone fishing?
    I dunno... The entire statement will execuite before program-flow gets to the next line, so pre-increment or post-incriment will only affect that one statement ++i<=strlen1;

    In any case, I recommend that you stick with the standard for-loop format... like hk_mp5kpdw's example:

    for(int i = 0; i < len1; ++i )

    ... unless you have a really good reason for doing otherwise.

    It is common to initialize and/or incriment more than one variable, but I would avoid doing anything in the for-loop's conditional statement that changes any values.

    Sometimes, I have to draw myself a picture of the array so I can visualize what's going on at the end of the loop. It's not always easy because you start at position-zero, you might be using >, <, <=, >=, etc, and you may have to take-into account the null termination, if it's a character arrray.

    FYI - A segmentation error has something to do with memory. When your program crashes, the first thing to look for is an array that is being written-to beyond its boundries. This usually happens in a loop. Screwed-up pointers can cause similar problems.

    If you can't spot the error by studying the code, you usually have to narrow-down the problem by commenting-out code 'till the program runs without crashing.

  8. #8
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,851
    How about just attaching your code, instead of pasting a url to a spamming host containing a file which doesn't exist...
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  9. #9
    Registered User
    Join Date
    Jan 2004
    Posts
    4
    Code:
    #include <iostream>
    using namespace std;
    
    //Function Prototypes
    bool Equal(char *str1,char *str2);
    
    int main()
    {
    	if(!Equal("test1", "test2"))
    	{
            cout << "test1 and test2 are different!" << endl;
    	}
    	else
    	{
    		cout << "test1 and test2 are the same!" << endl;
    	}
    	if(!Equal("monkeytest", "monkeytest"))
    	{
            cout << "monkeytest and monkeytest are different!" << endl;
    	}
    	else
    	{
    		cout << "monkeytest and monkeytest are the same!" << endl;
    	}
    	return(0);
    }
    
    
    bool Equal(char *str1,char *str2) //Is this the problem?
    {
    
     if(strlen(str1) != strlen(str2) )
            return(false);
     for(int i=-1;i<=(int)strlen(str1); ++i)
     {
    	 if(str1[i] != str2[i])
    	 {
    		 return(false);
    	 }
     }
    
     return(true);
    }
    works fine for me!

    edit ffcourse you can save strlen(str1) in an int straight away if you want to

  10. #10
    Registered User
    Join Date
    Jan 2004
    Posts
    10

    Post

    killgore:
    Ok,so it wasn't in Equal()... It's out now anyway..

    Salem:
    Better?

    DougDbug:
    In any case, i is increased to 0 (in the for statement) before it is used in any array operations. But I'll just go with the flow.
    Last edited by Keybone; 01-16-2004 at 05:01 AM.
    #include "Sig.h"
    #ifndef noob
    #define noob keybone
    #endif

  11. #11
    Registered User
    Join Date
    Jan 2004
    Posts
    10
    Damn it! cant attach anything when editing...
    Attached Files Attached Files
    • File Type: cpp 1.cpp (3.4 KB, 115 views)
    #include "Sig.h"
    #ifndef noob
    #define noob keybone
    #endif

  12. #12
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,851
    > Salem:
    > Better?

    Not really, cos no-one else has
    #include "Includes\inc.h"

    First obvious mistake
    while( success=false )
    == not =
    Better yet, use a boolean operator
    while ( !success )

    Perhaps less obvious error
    > struct user
    Have you ever changed this without deleting the previous data file created with the old version of the structure.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  13. #13
    Registered User
    Join Date
    Jan 2004
    Posts
    10
    Well, I've just bundled up all my includes into one file. I usually use the same set.

    The while...DOH! *Slaps myself* I knew that...at least I thought I did..

    The struct prob.. Could be... I'll try those things tonight.
    Thanks to everyone why bothered so far!
    #include "Sig.h"
    #ifndef noob
    #define noob keybone
    #endif

  14. #14
    Been here, done that.
    Join Date
    May 2003
    Posts
    1,161
    Originally posted by Keybone
    DougDbug:
    In any case, i is increased to 0 (in the for statement) before it is used in any array operations. But I'll just go with the flow.
    No it's not. The statement:
    Code:
    for(int i=-1;++i<=strlen1; )
    Says:
    1) start the loop at -1.
    2) When the loop is done,
    2a) increment i
    2b) test with strlen1
    3) Back to top.

    i is in fact -1 until the loop is finished.
    This statement
    Code:
    for(int i=0;i<=strlen1; i++)
    is the proper way to write a for statement
    Definition: Politics -- Latin, from
    poly meaning many and
    tics meaning blood sucking parasites
    -- Tom Smothers

  15. #15
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,893
    Actually the condition IS evalutated before the first run. But since this code is so uncommon, it should be the other. Simply for the sake of readability.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Confused with Structs and pointers
    By pc_doctor in forum C Programming
    Replies: 4
    Last Post: 03-27-2009, 08:08 PM
  2. Replies: 3
    Last Post: 04-19-2004, 07:42 PM
  3. Segmentation Fault printing an array of structs
    By ccoder01 in forum C Programming
    Replies: 1
    Last Post: 04-17-2004, 08:03 AM
  4. Passing pointers between functions
    By heygirls_uk in forum C Programming
    Replies: 5
    Last Post: 01-09-2004, 06:58 PM
  5. Contest Results - May 27, 2002
    By ygfperson in forum A Brief History of Cprogramming.com
    Replies: 18
    Last Post: 06-18-2002, 02:27 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21