Programming in C - returning a string from a function

[Hi P]

I'm following a course (tutorial online) on C, one of the small challenges was to create a function that concatenate two strings, without using the C libraries.

 

The problem is, my code compiles but doesn't print anything at all, and I don't know why

 

(I made the variable names as verbose as possible, hoping to make it easier to read)

 

Spoiler

#include <stdio.h>

char *concatenate(char *first_string, char *second_string);

int main(void)
{
    char *part_A = "Hello ",
         *part_B = "world";

    printf("%s", concatenate(part_A, part_B));
}

char *concatenate(char *first_string, char *second_string)
{
    int index = 0;
    char *combined_parts;

  	// insert characters from the first string into the new string
    for (int character = 0;; character++, index++)
    {
        if (first_string[character] == '\0')
            break;

        combined_parts[index] = first_string[character];
    }

  	// insert characters from the second string into the new string
    for (int character = 0;; character++, index++)
    {
        if (second_string[character] == '\0')
            break;

        combined_parts[index] = second_string[character];
    }

    return combined_parts;
}

 

 

1.- What's wrong with it? (it compiles but does nothing)

2.- How do I successfully return a string?

 

Thank you very much

 

 

[Gronnie]

I see a couple of problems with combined_parts:

1. It's on the stack (meaning once you return it is out of scope).

2. You don't give it a size (how much memory is/needs to be allocated?)

[DevBlox]

You don't allocate combined_parts, you only declare a pointer which points to whatever. Dangerous code, you should be getting an access violation error (not sure how's it running even). You should decide whether you want concatinate to allocate memory or not (does not really matter if you're just trying things out). Then allocate a chunk of memory (malloc) long enough to fit both strings, other than that, at a glance, you should be fine.

 

In C you don't really "return a string", you return a pointer to the start of the string, which is what char * essentially is, same when passing a string to function.

 

When you allocate with malloc, you allocate on the heap, the memory will be immune to scope change. Just don't forget to free it afterwards

[IAmAndre]

As it's been said before, you need to rethink the way you allocate memory as you're even lucky that this code runs at all. You should have gotten either a complication issue or a segmentation fault while running it.

With that said, you should also check your semantics: those for/if/break statements look bad. Just initiate your variables and use a while statement. It'll look a lot cleaner.

[Hi P]

1 hour ago, Gronnie said:

1. It's on the stack (meaning once you return it is out of scope).

2. You don't give it a size (how much memory is/needs to be allocated?)

 

45 minutes ago, DevBlox said:

Allocate a chunk of memory (malloc) long enough to fit both strings, in C you don't really "return a string", you return a pointer to the start of the string, which is what char * essentially is, same when passing a string to function.

Thank you guys!

 

I haven't reached the pointer chapter though, but I got a brief introduction to them on CS50, I completely forgot about malloc and free

 

Edit:

char *combined_parts = malloc(sizeof(char) * 12);

 

Wait nevermind, I got a problem, why does it return the string + garbage values?

 

I'm getting this:

Hello worldriverDa┬L=    0

[Unimportant]

Undefined behavior. You alloc a single byte and then proceed to write additional bytes in the memory following the allocated byte, memory that does not belong to you. The C standard leaves this situation undefined. You should not do it but anything can happen if you do, including the program working normally. By leaving such situations undefined compiler implementers don't have to add various checks and tests into your code, which is the main reason C and C++ can produce such fast code.

 

[Hi P]

1 minute ago, Unimportant said:

Undefined behavior. You alloc a single byte and then proceed to write additional bytes in the memory following the allocated byte, memory that does not belong to you. The C standard leaves this situation undefined. You should not do it but anything can happen if you do, including the program working normally. By leaving such situations undefined compiler implementers don't have to add various checks and tests into your code, which is the main reason C and C++ can produce such fast code.

 

Nevermind, I edited the post, it began to print out garbage values, and I can't get rid of them (I already increased malloc size)

 

Any idea why?

[Unimportant]

Probably because you forgot to terminate the string with a 0. You need to allocate 1 byte more then the string length and put a 0 at the end. The 0 marks the end so functions like printf know where to stop. ( integer value 0, not character '0').

[Hi P]

25 minutes ago, Unimportant said:

Probably because you forgot to terminate the string with a 0. You need to allocate 1 byte more then the string length and put a 0 at the end. The 0 marks the end so functions like printf know where to stop. ( integer value 0, not character '0').

Thank you very much, it worked

 

[Mira Yurizaki]

A bit of a suggestion to possibly make it easier to deal with memory management, should you ever do something like this beyond academic purposes. Make the function take another  argument that is a pointer to where the string will live. While the general rule for dynamic memory allocation is you should have a free for every corresponding malloc call, it can be harder to keep track of this if something else made the malloc call for you, but doesn't clean up after itself in some form or fashion. Otherwise if you do keep malloc in the function, you need to make it clear somehow that the memory from the pointer it returns needs to be freed up.

 

For additional safety, you can have additional arguments to limit how many bytes are copied.

[Hi P]

7 hours ago, Mira Yurizaki said:

A bit of a suggestion to possibly make it easier to deal with memory management, should you ever do something like this beyond academic purposes. Make the function take another  argument that is a pointer to where the string will live. While the general rule for dynamic memory allocation is you should have a free for every corresponding malloc call, it can be harder to keep track of this if something else made the malloc call for you, but doesn't clean up after itself in some form or fashion. Otherwise if you do keep malloc in the function, you need to make it clear somehow that the memory from the pointer it returns needs to be freed up.

 

For additional safety, you can have additional arguments to limit how many bytes are copied.

Thank you

 

Let me see if I understood this correctly, I shouldn't use malloc inside a function for a variable that I plan to return, since by doing so I won't be able to free the memory, so instead I should declare a variable in main, call malloc on it and use the variable address as a parameter for the function, which would allow me to free the memory once I'm done with it

 

 

(let me know if I got the wrong idea)

[Mira Yurizaki]

1 hour ago, Hi P said:

Thank you

 

Let me see if I understood this correctly, I shouldn't use malloc inside a function for a variable that I plan to return, since by doing so I won't be able to free the memory, so instead I should declare a variable in main, call malloc on it and use the variable address as a parameter for the function, which would allow me to free the memory once I'm done with it

 

 

(let me know if I got the wrong idea)

You can call free on a pointer as long as points to a memory location that malloc has given you. The issue with calling malloc inside the function and having it return the pointer that malloc gave is that it can make tracking down what you need to free later harder. It's not necessarily a problem with an app of this size, but something to remember.

 

Also another thing to note is malloc can fail, in which case it returns a NULL pointer. So you should be checking for that too. This is also why you don't want the malloc call in the function. Though free is fine taking a NULL pointer, so you can call that unconditionally.

[Unimportant]

You can find some inspiration in the C standard library function "strcat", which performs the same function:

char *strcat( char *dest, const char *src );
	

Quote

Appends a copy of the character string pointed to by src to the end of the character string pointed to by dest. The character src[0] replaces the null terminator at the end of dest. The resulting byte string is null-terminated.

The behavior is undefined if the destination array is not large enough for the contents of both src and dest and the terminating null character.

The behavior is undefined if the strings overlap.

Source: https://en.cppreference.com/w/cpp/string/byte/strcat

 

Thus, the string pointed to by "src" is appended to the string pointed to by "dest". It's upto the caller to make sure "dest" points to a memory block long enough to hold the resulting string + null terminator. This removes the immediate need for dynamic memory allocation and shifts the choice to the caller, which is preferable, a plain static array may suffice. It's not up to the library writer to make/force such choices.

 

You may want to model your own implementation of string concatenation in the same way.

[wasab]

I don't think strcat() in the c standard library returns anything. It simply copies whatever is in the 2nd string into the memory of the first. If you want to do your own custom strcat(), maybe do the same? Return nothing and simply copy over. 

 

Edit: nevermind. It does return the pointer to the string.