Thread: Uncertain why program hangs; Simple string copy with pointers

Code:

#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>	/* Include the stand boolean header */
#include <math.h>
#include "share.h"        /* Custom header of no affect to problem */

void cpy(char *original, char *copy)
{
	while (*original != '\0')
		*copy++ = *original++;
}

int
main(VOID)
{
	char phrase[] = "Blessed be the Lord!";

	char *copy = "";

	cpy(phrase, copy);
	printf ("%s", copy);
	return 0;
}

I have not debugged this programing using gdb though I have ascertained the problem lay with the pointer assignment in the function.

Hints: modifying a string literal results in undefined behaviour; watch out for your null character.

By the way, unless you have special reason to use a macro, just use void instead of VOID. Showing the headers included would also be a good idea.

Another hint- make sure you've allocated enough memory in "copy" to store what you want to put there.

Originally Posted by Imanuel

Code:

#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>	/* Include the stand boolean header */
#include <math.h>
#include "share.h"        /* Custom header of no affect to problem */

void cpy(char *original, char *copy)
{
	while (*original != '\0')
		*copy++ = *original++;
}

int
main(VOID)
{
	char phrase[] = "Blessed be the Lord!";

	char *copy = "";

	cpy(phrase, copy);
	printf ("%s", copy);
	return 0;
}

I have not debugged this programing using gdb though I have ascertained the problem lay with the pointer assignment in the function.

1.VOID is not a default C keyword, void is. There is a difference.
2. char* copy = ""; makes a string literal, just like char phrase[]=".." does. In order to do what you want you need to declare a char* and then get memory for it with malloc. Note: You could just define a char[] with some default size in it.
3. Your while loop as written does not place the '\0' char at the end of your copied string (assuming that would work). Note: C style strings need to be terminated with '\0'. Look carefully at your loop logic.

Originally Posted by Imanuel

I used char *copy = malloc(100) and it worked fine. I never though I actually needed to preallocate the memory for the pointer. Thank you.

*copy = malloc(100) is fine and correct, since a char always has size 1, but in the interests of clarity you might consider *copy = malloc( 100 * sizeof(char) ) since this will make it clear to anyone reading your code what kind of thing is supposed to go into copy. Writing code that is self-commenting as possible is good programming practice.

Originally Posted by TheBigH

*copy = malloc(100) is fine and correct, since a char always has size 1, but in the interests of clarity you might consider *copy = malloc( 100 * sizeof(char) ) since this will make it clear to anyone reading your code what kind of thing is supposed to go into copy.

That is quite clear since the type name is already on the same line. If you really do want to have the sizeof, then I suggest:

Code:

char *copy = malloc(100 * sizeof(*copy));

You're right, I overlooked the type name. Disregard.