Thread: [sig fault] C-string appender

  1. #1
    Registered User
    Join Date
    Nov 2010
    Posts
    3

    [sig fault] C-string appender

    Hello,

    I started programming in c a while ago. And to understand things better, i made a my own string appender (append b to a). I got version 1.0 working:
    Code:
    char * append(char * dest, char * src)
    {
      char * buffer = malloc(strlen(dest) + strlen(src) + 1);
      unsigned int i = 0;
      unsigned int x = 0;
    
      for(; dest[i] != '\0'; i++) buffer[i] = dest[i];
      
      x = i;
      for(; src[i-x] != '\0'; i++) buffer[i] = src[i-x];
     
      buffer[i] = '\0';
    
      return buffer;
    
    }
    but version 2.0, where i give the adres of the buffer in the header (so x = append(a, b) can be replaced with append(a, b)), is not working.. it gives me a sig fault.. and i can't find where it goes wrong with my memory..:

    I tried this:
    Code:
    void append(char ** buffer, char * src)
    {
      int i;
      for(i = 0; i <= strlen(src); i++) buffer[i] = &src[i];
    }
    and this..
    Code:
    void append(char ** buffer, char * src)
    {
      int i;
      for(i = 0; i <= strlen(src); i++) *buffer[i] = src[i];
    }
    The function call in my main function looks like this:

    Code:
    int main()
    {
        char * str1 = "Hello";
    
        char * buffer = malloc(strlen(str1) + 1);
        append(&buffer, str1);
    
    
        printf("%s", buffer);
        free(buffer);
        return 0;
     }
    Can anyone tell me where, and why it goes wrong, and maby even try to explain how to fix?

    Thanks in advance!
    Last edited by Bietje; 11-19-2010 at 11:15 AM.

  2. #2
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    I haven't compiled anything yet or debugged it, but I would bet it is because you never gave buffer any initial data, and malloc doesn't guarantee that the memory is zero-filled. Thus, there is no null at the beginning of buffer, so your append function runs way off the end of dest before even trying to append src. Simply do a buffer[0] = '\0'; after your malloc.

    A few notes about your append function:
    1. Put the initial condition in your for loop, like so: for (i = 0; i < end; i++), for readability.
    2. Skip the assignment of *dest[i] = *dest[i]; Just have an empty loop body.
    3. Pass in a length parameter (for the total length of dest), to avoid such overflows.
    4. Make your final loop something like for (x = 0; src[x] != '\0' && i < len - 1; i++, x++). Then i will be your index into dest, and x your index into src.

  3. #3
    Banned
    Join Date
    Aug 2010
    Location
    Ontario Canada
    Posts
    9,547
    Did you try this...
    Code:
    void append(char buffer, char src)
    { int i;
      for(i = 0; i <= strlen(src); i++) 
        buffer[i] = src[i]; }
    Should copy src to buffer at it's beginning.

    Code:
    void append(char buffer, char src)
      {  int o = strlen(buffer) - 1;
          for (int i = 0; i < strlen(src) + 1; i++)
              buffer[i + o] = src[i]; }
    The above should do a true append.
    Last edited by CommonTater; 11-19-2010 at 12:55 PM. Reason: Laserlight cauge me at a mistake...

  4. #4
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Quote Originally Posted by Bietje
    And to understand things better, i made a my own string appender (append b to a).
    Are you trying to write your own version of strcat? Note that strcat just works with the existing char arrays rather than use dynamic memory allocation.

    Quote Originally Posted by CommonTater
    Did you try this...
    The parameter types are wrong, and even the second example does a copy not an append because of a typo error.
    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

  5. #5
    Banned
    Join Date
    Aug 2010
    Location
    Ontario Canada
    Posts
    9,547
    Quote Originally Posted by laserlight View Post
    Are you trying to write your own version of strcat? Note that strcat just works with the existing char arrays rather than use dynamic memory allocation.


    The parameter types are wrong, and even the second example does a copy not an append because of a typo error.
    Wow... time for a nice hot coffee... I need a break.... you're right, silly me.

    How are the parameter types wrong?
    Last edited by CommonTater; 11-19-2010 at 12:57 PM.

  6. #6
    Gawking at stupidity
    Join Date
    Jul 2004
    Location
    Oregon, USA
    Posts
    3,218
    Quote Originally Posted by CommonTater View Post
    Wow... time for a nice hot coffee... I need a break.... you're right, silly me.

    How are the parameter types wrong?
    They should be type char *, not char.

    Also, using strlen() in a loop isn't very efficient. I'd recommend maybe using a variable that holds the length instead.
    If you understand what you're doing, you're not learning anything.

  7. #7
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    Quote Originally Posted by CommonTater View Post
    How are the parameter types wrong?
    You used char instead of char *.

    @Bietje:
    Since you have the idea of the append function down, here is how I would write it (including null pointer and overflow checks. This is untested, but shows the general idea:
    Code:
    int append(char *dest, const char *src, int len)
    {
        int i, j;
    
        if (!dest || !src) {
            return BAD_PARAM;
        }
    
        j = strlen(dest);
    
        // check that there is room to append src and a null character
        if (j + strlen(src) + 1 > len) {
            return OVERFLOW;
        }
    
        for (i = 0; src[i] && j < len - 1; i++, j++) {
            dest[j] = src[i];
        }
        dest[j++] = '\0';
    
        // return zero on success
        return 0;
    }
    You could alternatively return the number of characters copied (i.e. i), or the new length of dest (i.e. j) on success, so long as your error codes are negative numbers.

  8. #8
    Registered User
    Join Date
    Oct 2008
    Posts
    1,262
    I'm more of a fan of code like this:
    Code:
    void append(char *dst, const char *src)
    {
      dst += strlen(dst);
      while(*dst++ = *src++);
    }
    but of course, that's just me...

  9. #9
    Gawking at stupidity
    Join Date
    Jul 2004
    Location
    Oregon, USA
    Posts
    3,218
    If you guys aren't going to worry about memory management, then just:
    Code:
    void append(char *dst, char *src)
    {
      memcpy(dst + strlen(dst), src, strlen(src) + 1);
    }
    or hell:
    Code:
    void append(char *dst, char *src)
    {
      strcat(dst, src);
    }
    Why reinvent the wheel?
    If you understand what you're doing, you're not learning anything.

  10. #10
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    Quote Originally Posted by EVOEx View Post
    I'm more of a fan of code like this:
    Code:
    void append(char *dst, const char *src)
    {
      dst += strlen(dst);
      while(*dst++ = *src++);
    }
    but of course, that's just me...
    You're going to scare the poor guy/girl off with your crazy magic pointer ju-ju!

  11. #11
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Quote Originally Posted by itsme86
    Why reinvent the wheel?
    Because:
    Quote Originally Posted by Bietje
    And to understand things better, i made a my own string appender (append b to a).
    Notice that I asked about this in post #4.
    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

  12. #12
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    Code:
    void append(char ** buffer, char * src)
    {
      int i;
      for(i = 0; i <= strlen(src); i++) (*buffer)[i] = src[i];
    }
    Watch the ( ), they're important.

    It's the difference between index and then dereference (wrong), vs. dereference and then index (right).
    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.

  13. #13
    Banned
    Join Date
    Aug 2010
    Location
    Ontario Canada
    Posts
    9,547
    Quote Originally Posted by itsme86 View Post
    They should be type char *, not char.

    Also, using strlen() in a loop isn't very efficient. I'd recommend maybe using a variable that holds the length instead.
    duh-oh.... I should have known that.

    This is where Windows spoils me... In a windows proggy I would have just automatically used PCHAR... PCHAR = char* and I should have known that.

    Good point about strlen() in a loop condition... That one's noted for future.

  14. #14
    Registered User
    Join Date
    Nov 2010
    Posts
    3
    Thanks for all your reactions! I'm going to try some of the sugested things now

  15. #15
    Registered User
    Join Date
    Nov 2010
    Posts
    3
    Hello guys.. Here I am again.. i tested almost every sugestion you guys did.. thanks for them + the tips.
    If i strip of all the nullpointer checks and stuff then i have these two functions:
    Code:
    void append(char * buffer, char * src)
    {
      int i, n = strlen(buffer);
      for(i = 0; i <= strlen(src); i++) buffer[i+n] = src[i];
    }
    called with
    Code:
    append(buffer, src);
    and
    Code:
    void append(char ** buffer, char * src)
    {
      int i, n = strlen(*buffer);
      for(i = 0; i <= strlen(src); i++) (*buffer)[i+n] = src[i];
    }
    called with
    Code:
    append(&buffer, src);
    Both functions seem to do the same thing.. the thi'ng I wanted it did.. But is there any difference (yes i know the difference (i think i do.. xd) at the second function I give a reference to a c-style string in stead of a pointer) but are there any performence differences? Or doesn't it matter at all..?

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. OOP Question DB Access Wrapper Classes
    By digioz in forum C# Programming
    Replies: 2
    Last Post: 09-07-2008, 04:30 PM
  2. Message class ** Need help befor 12am tonight**
    By TransformedBG in forum C++ Programming
    Replies: 1
    Last Post: 11-29-2006, 11:03 PM
  3. Another overloading "<<" problem
    By alphaoide in forum C++ Programming
    Replies: 18
    Last Post: 09-30-2003, 10:32 AM
  4. Classes inheretance problem...
    By NANO in forum C++ Programming
    Replies: 12
    Last Post: 12-09-2002, 03:23 PM
  5. Warnings, warnings, warnings?
    By spentdome in forum C Programming
    Replies: 25
    Last Post: 05-27-2002, 06:49 PM