0

This is a hw assignment that I am having a lot of difficulty with. I'm tasked with creating my own function that accepts two strings and will concatenate the two strings together and will return a character pointer to the new string. This is what I currently have:

char * my_strcat(char p[], char t[]) {
    printf("Made it into my_strcat.\n");

    int size1;
    int size2;

    size1 = my_strlen(p);
    size2 = my_strlen(t);

    int size3;
    size3 = size1 + size2 +1;

    printf("This many characters allocated for in memory: %i\n", size3);

    char* MemLoc = (char *)malloc(sizeof(char)*(size1+size2+1));
    char* BookMark = MemLoc;

    printf("Address of MemLoc: %p\n", MemLoc);
    printf("Address of BookMark: %p\n", BookMark);

    int count = 0;

    while(count < size1) {
        *BookMark = p[count];
        printf("Address of MemLoc: %p\n", MemLoc);
        printf("Latest char: %c\n", *MemLoc);
        count++;
        BookMark++;
        printf("Address of MemLoc: %p\n", MemLoc);
    }

    count = 0;

    while(count < size2) {
        *BookMark = t[count];
        printf("Latest char: %c\n", *MemLoc);
        count++;
        BookMark++;
    }

    *BookMark = '\0';
    printf("Concatenated string: %s\n", MemLoc);

    return MemLoc;
}

I have a lot of print statements in there trying to determine my error but I still can't pin it down. Depending on what type I print as, I am getting "nil", "null", and nothing for the last print statement. The last print statement (in my opinion) should be printing the final concatenated string.

Also, I had to construct the my_strlen method to calculate string length. This is working correctly and returns the correct length values of the strings sent in. Any suggestions for fixing this method?

  • 2
    Why the +4 ? I think it should be BookMark++ – Ferenc Deak Apr 27 '14 at 15:15
  • I tried that and tested it by printing the memory location and it was only incrementing the address by 1. I thought it was supposed to increase the address by the size of the data type also but it did not seem to be doing that. – user2956552 Apr 27 '14 at 15:17
  • 1
    The size of a char is 1. The size of a char* is 4 (depending on your architecture). And you always print MemLoc which is not changing. – Ferenc Deak Apr 27 '14 at 15:20
  • 1
    `BookMark` points to `char` and the size of a `char` **is** 1. – alk Apr 27 '14 at 15:20
  • 2
    Whe loop for while `count < size1 - 1`? As you use `strlen` to get `size1` and `size2` that length already is without the terminator. Either do `count < size1` (most usual) or `size <= size - 1`. – Some programmer dude Apr 27 '14 at 15:23
  • Ok. Changed BookMark to be incremented by 1. I am still having the same error when printing the final new string by referencing the dereferenced MemLoc. – user2956552 Apr 27 '14 at 15:23
  • 2
    Also, in C you [should not cast the return of `malloc`](http://stackoverflow.com/a/605858/440558). – Some programmer dude Apr 27 '14 at 15:24
  • 1
    Your last `printf` should have `%s` because `MemLoc` is a `char *`, not a `char`. –  Apr 27 '14 at 15:25
  • 1
    You print `*MemLoc` as the last character, as well as its address, but you put the character at `*Bookmark` and also it's pointer you increase. `*MemLoc` will always be the first character. – Some programmer dude Apr 27 '14 at 15:26
  • 2
    You should consider turning up your compiler's warning level. With GCC and Clang, it is -Wall that will help. Then you just need to understand what the compiler tells you. It is usually very helpful. –  Apr 27 '14 at 15:28
  • @JoachimPileborg, at the end of the processing will not *BookMark only point to the last character? I thought I needed two references to the start of the string, one to manipulate, and one to reference to print the final string. – user2956552 Apr 27 '14 at 15:29
  • 1
    @ChronoKitsune, Thanks for the tip. That is definitely very useful. – user2956552 Apr 27 '14 at 15:30
  • 1
    `MemLoc` will always point to the beginning of the string, while `BookMark` points to the end of the string, so printing `*MemLoc` in a loop will always print the same character, the first. – Some programmer dude Apr 27 '14 at 15:30
  • You keep printing the value of `MemLoc` in your `while` loops, but the loops only modify `BookMark`. Not sure what you're expecting to ser. – lurker Apr 27 '14 at 15:31
  • Suggest `char * my_strcat(char p[], char t[])` --> `char * my_strcat(const char p[], const char t[])`. – chux - Reinstate Monica Apr 27 '14 at 15:42
  • In the future, do not edit your post - maybe _append_ relevant info as it comes up. What you have now looks odd as it has few problems and does not make sense with all the comments folks who have helped you. – chux - Reinstate Monica Apr 27 '14 at 15:44
  • C uses `size_t` in many places where this code uses `int`. For greater compatibility, use `size_t my_strlen(const char *s)` and `size_t size1;`, etc. in this code. – chux - Reinstate Monica Apr 27 '14 at 15:48
  • Noted @chux. Also reference my comment on the Answer post. – user2956552 Apr 27 '14 at 15:49

1 Answers1

1

You might use a printf of the form

    printf("built string is '%.*s'\n",size3,MemLoc);

or

    printf("build string is '%.10s'\n",MemLoc);

to help you debug. By using the precision limit on the %s format specifier you can prevent the format conversion from running off of the end of the unterminated partially built string. Since its just debug and you probably know the length of your test case there is really no harm in using the fixed length version.

For help in this debugging you would probably also want to

    memset(MemLoc,'#',size3);

after you malloc it and before you start construction. This can help avoid confusion with garbage that was already in memory vs. what you are doing to it.

With that initialization and scattering the prints around, I'm hopeful you will be able to debug your problems.

I agree with friz's comment that the +4 for stepping through the string doesn't make sense.

dennis
  • 221
  • 1
  • 5
  • Thanks. I solved the error(s). Apparently I "can't answer my own question" for another 8 hours. I will be sure to post my solution then. Thanks for the help everyone. – user2956552 Apr 27 '14 at 15:43