Concatenation without string.h - program crashes

Code:

---

void my_strcat (char destination[], const char source[])
{
    int i = 0, j = 0;
    while (destination[i] != '\0') i++;
    destination[i++] = ' ';
    while (source[j - 1] != '\0' || !j) destination[i++] = source[j++];
    puts(destination);
}

---

Why does the above code crash for destination = "abc" and source = "abcjjj"? I'd truly appreciate some advice.

Well think about it, if you don't allocate enough space in destination for the source, it will crash. You can't fix a big piece of data in a small container, otherwise it will crash. Just like strcat, you shouldn't try to put bigger amounts of things into a smaller arrays.

How may it be emended? How may I prevent it from crashing?

The first time through, your program is going to try access source outside its bounds. j is initialised to 0, you call j-1 which will be before the start of your source string, you have no guarantees what will be in this value.

Also, were you told to use that type of construct for a while loop? The way I read that, your first loop will loop iterate until it encounters a '\0' character then assign ' ' to the space after that one. Is there any reason to expect your destination array will contain any '\0' characters?

Quote:

---

Originally Posted by AAnderson

The first time through, your program is going to try access source outside its bounds. j is initialised to 0, you call j-1 which will be before the start of your source string, you have no guarantees what will be in this value.

Also, were you told to use that type of construct for a while loop? The way I read that, your first loop will loop iterate until it encounters a '\0' character then assign ' ' to the space after that one. Is there any reason to expect your destination array will contain any '\0' characters?

---

We were told to presume all strings end with '\0'.
How would you suggest I fix this?

This looks wrong:

Code:

---

while (source[j - 1] != '\0' || !j)

---

I would expect you to iterate over the "source" string in the same manner as you first iterate over the "destination" string, except that you will be copying characters.

Also, it would be good if you posted the code that tests this function to verify that the problem isn't elsewhere.

Code:

---

int main()
{
    char arr1[] = "abc";
    char arr2[] = "abcffffff";
    my_strcat(arr1, arr2);
    return 0;
}




void my_strcat (char destination[], const char source[])
{
    int i = 0, j = 0;
    while (destination[i] != '\0') i++;
    destination[i++] = ' ';
    while (source[j - 1] != '\0' || !j) destination[i++] = source[j++];
    printf("%s\n", destination);
}

---

What do you think, laserlight?

It is still wrong: source[j - 1] accesses source out of bounds when j == 0.

Code:

---

int main()
{
    char arr1[] = "abc";

---

This allocates memory for 4 characters. Later you are trying to add more characters but that doesn't work. That's like having a 4 litre bucket full of water and then trying to add more water to it. The bucket will overflow and the same happens with the array.
You need an array which has enough room for all the characters in arr1 and all the characters in arr2 (and don't forget to count '\0').

Bye, Andreas

Quote:

---

Originally Posted by AndiPersti

Code:

---

int main()
{
    char arr1[] = "abc";

---

This allocates memory for 4 characters. Later you are trying to add more characters but that doesn't work. That's like having a 4 litre bucket full of water and then trying to add more water to it. The bucket will overflow and the same happens with the array.
You need an array which has enough room for all the characters in arr1 and all the characters in arr2 (and don't forget to count '\0').

Bye, Andreas

---

But in any case, due to the concatenation itself, destination is going to contain more characters than it was originally set up for, right? I mean, even if source is merely one char long it would still be added to it so that destination becomes longer. This is what puzzles me! Would you care to explain?

And laserlight, it doesn't really matter whether I access j-1 or not because of the "||!j". Thus, the procedure ignores whatever is in that location for an out-of-bound j. I could easily "correct" this by adding a condition in parenthesis like ((source[j - 1] != '\0' && j != 0)|| !j)
But which would still not save it from crashing!
How may I fix the crashing?

Quote:

---

Originally Posted by peripatein

But in any case, due to the concatenation itself, destination is going to contain more characters than it was originally set up for, right? I mean, even if source is merely one char long it would still be added to it so that destination becomes longer.

---

You are mistaken: whatever you defined the array length to be, that is the array length. It does not change just because you attempt to access elements of the array that do not exist. Hence, the solution is to create an array large enough to hold the result, e.g.,

Code:

---

char arr1[13] = "abc";

---

So, the length of the string is 3, but the number of elements of the array is 13. Since you will append 9 characters to the string, 3 + 9 + 1 (null character) = 13, hence this is okay.

Quote:

---

Originally Posted by peripatein

And laserlight, it doesn't really matter whether I access j-1 or not because of the "||!j". Thus, the procedure ignores whatever is in that location for an out-of-bound j.

---

You are mistaken: the check for !j comes after you access source[j - 1]. Therefore, you access the array out of bounds.

Quote:

---

Originally Posted by peripatein

I could easily "correct" this by adding a condition in parenthesis like ((source[j - 1] != '\0' && j != 0)|| !j)

---

That would make no difference since you still access source[j - 1] when j == 0. The thing is, why check for source[j - 1] instead of source[j]?

Quote:

---

Originally Posted by peripatein

How may I fix the crashing?

---

Fix both the problems mentioned to you concerning array length and accessing the array out of bounds.

But, laserlight, how would I then make the procedure add the final '\0' of source to destination? I do not wish to add an extra line just for that one enclosing char but would like it to be done as part of the loop.

Quote:

---

Originally Posted by peripatein

I do not wish to add an extra line just for that one enclosing char but would like it to be done as part of the loop.

---

Concentrate on writing a working function first.

Bye, Andreas

Quote:

---

Originally Posted by peripatein

how would I then make the procedure add the final '\0' of source to destination?

---

When you are done with the copying, i would contain the index of the character that should be the null character.

Quote:

---

Originally Posted by peripatein

I do not wish to add an extra line just for that one enclosing char but would like it to be done as part of the loop.

---

It can be done, but it requires some code dissection to understand:

Code:

---

while (destination[i++] = source[j++]);

---

By the way, I realised that you append a space before appending the "source" string, so you need to account for this space as well.

Laserlight, what if I inverted the order of my condition, namely:

Code:

---

while ((j != 0 && source[j - 1] != '\0') || !j) destination[i++] = source[j++];

---

Wouldn't that prevent the procedure from accessing a location out of bounds?

And regardless, suppose I wrote the following:

Code:

---

int main()
{
    char arr1[60] = "abc";
    char arr2[] = "abcffffff fldsflksfls";
    my_strcat(arr1, arr2);
    return 0;
}
void my_strcat (char destination[], const char source[])
{
    int i = 0, j = 0;
    while (destination[i] != '\0') i++;
    destination[i++] = ' ';
    while (source[j] != '\0') destination[i++] = source[j++];
    printf("%s\n", destination);
}

---

Would you consider that to be a complete solution or is anything missing/unaccounted for?

Quote:

---

Originally Posted by peripatein

Wouldn't that prevent the procedure from accessing a location out of bounds?

---

Yes, but is is also more difficult to understand than just checking for source[j] != '\0' or even using the concise loop that I showed you.

Quote:

---

Originally Posted by peripatein

Would you consider that to be a complete solution or is anything missing/unaccounted for?

---

Looks fine, but of course, you should be printing outside of my_strcat.

Quote:

---

Originally Posted by laserlight

Yes, but is is also more difficult to understand than just checking for source[j] != '\0' or even using the concise loop that I showed you.


Looks fine, but of course, you should be printing outside of my_strcat.

---

Were you referring to my latest attempt, namely while (source[j] != '\0') destination[i++] = source[j++]; ? Or rather to the inversion of order?
Moreover, is there no need to "manually" add '\0' to destination at the end? Is that taken care of by the loop?

Code:

---

void my_strcat (char destination[], const char source[])
{
    int i = 0, j = 0;
    while (destination[i] != '\0') i++;
    destination[i++] = ' ';
    while (source[j] != '\0') destination[i++] = source[j++];
    printf("%s\n", destination);
}

---

You forget to terminate "destination". Your loop ends when source[j] == '\0' thus you are not copying '\0' to "destination".

Bye, Andreas

In that case, I could just stick to my original code, writing thus:

Code:

---

void my_strcat (char destination[], const char source[])
{
        int i = 0, j = 0;
        while (destination[i] != '\0') i++;
        destination[i++] = ' ';
        while ((j != 0 && source[j - 1] != '\0') || !j) destination[i++] = source[j++];
}

---

right?

You cannot do that source[j - 1] is invalid when j is 0.
Kurt
EDIT: I see it now, could work but it's a really strange solution

The main real goal in programming is to write maintainable code.

You failed in that goal on you last post of code on this line.

Code:

---

while ((j != 0 && source[j - 1] != '\0') || !j) destination[i++] = source[j++];

---

Much more complex than will be easy to maintain.

Tim S.

But it doesn't access that location once I include the condition j != 0 !! Why is that not maintainable?

Quote:

---

Originally Posted by peripatein

But it doesn't access that location once I include the condition j != 0 !! Why is that not maintainable?

---

What functionality does the " || !j " add to your code?

Tim S.

Quote:

---

Originally Posted by stahta01

What functionality does the " || !j " add to your code?

Tim S.

---

Without that the loop wouldn't run at all. I said it "strange solution" just to save one line of code.
Kurt

I personally find it more sophisticated. Yes, more cumbersome and complicated perhaps, strange you might add, yet this way the string (viz. destination) is added the enclosing '\0' by the while loop itself.

I could have likewise written it thus:

Code:

---

while (!j || source[j - 1] != '\0') destination[i++] = source[j++];

---

Don't write code to be unnecessarily clever; write code to be clear. Be considerate to the person who will maintain your code when you're gone. Even if you're just learning, get into this habit.

Quote:

---

Originally Posted by peripatein

Were you referring to my latest attempt, namely while (source[j] != '\0') destination[i++] = source[j++]; ? Or rather to the inversion of order?

---

My comments correspond to what I quoted, i.e., the first comment for the first snippet; the second comment for the second snippet.

Quote:

---

Originally Posted by peripatein

Moreover, is there no need to "manually" add '\0' to destination at the end? Is that taken care of by the loop?

---

You should add the null character at the end, and in fact I implied that in my earlier posts. My mistake on saying that your second code snippet is fine though, but in my defense I was too busy checking your first code snippet for correctness and did not notice that you missed that part out.

Quote:

---

Originally Posted by peripatein

I personally find it more sophisticated. Yes, more cumbersome and complicated perhaps, strange you might add, yet this way the string (viz. destination) is added the enclosing '\0' by the while loop itself.

---

It is obfuscated, not sophisticated. Even the method that I showed you in post #14 is somewhat obfuscated, but at least it is well known. With it, you can simplify your function to:

Code:

---

void my_strcat(char destination[], const char source[])
{
    int i = 0, j = 0;
    while (destination[i] != '\0') i++;
    destination[i++] = ' ';
    while (destination[i++] = source[j++]);
}

---