Trouble copying a string in C?

Question

I am trying to copy the String an char pointer to another ?

I tried the code given below .compiled successfully but output is blank(no output) .

a) Where I was wrong?where my programming logic getting fail?

b) How can I improve this code to get desired output?

    void main()
    {
     char *p="krishna";
     char *q;

      while(*p!='\0')
      {
       *q++=*p++;
      }
      printf("%s",q);
      getch();
    }

Answer 1

a) Where I was wrong?where my programming logic getting fail?

Well, you did several things incorrectly. For one, void main is non-standard; the return type for main should be int.

That being said, the real issue you're looking for has to do with the fact that q is uninitialized, yet you attempt to still copy to memory through it (which is undefined behavior). To correct that, try allocating q, e.g.

char *q = malloc(8);

Note you must then later also take care to free the memory allocated here.

Aside from that, you're forgetting to copy the NUL terminator, too.

*q = 0;

... after your copying loop. You also are printing q after incrementing the pointer, so it will no longer be at the head of the string by the time of your printf call. You should store a copy of the head in another variable. Additionally, be careful using a plain printf without any new-lines, as the stream may be buffered and thus could remain unflushed -- use an explicit fflush(stdout);

---

b) How can I improve this code to get desired output?

Well, the first most straight-forward improvement I can think of is to use strcpy.

#include <stdio.h>

main() {
  const char p[] = "krishna";
  char q[sizeof p];
  strcpy(q, p);
  puts(q);
  getchar();
  return 0;
}

Answer 2

In this example you provided, you are not allocating memory for the new array of characters, but instead you are just copying over characters into an unknown buffer. This could cause all sorts of problems (read about buffer overflows and attacks). Furthermore, once you are done copying over the characters into the unknown buffer, this buffer is no longer pointing to the beginning of the string, but instead the end of the string. If your idea is to copy over the character array as a string in C, you can do so with the following code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
void main()
{
  char *p="krishna";
  char *q = (char*) malloc(sizeof(char) * (strlen(p) + 1));

  strcpy(q, p);

  printf("%s", q);
  getchar();
  free(q);
  return 0;
}

In most scenarios, the extra byte at the end of the array for the NULL character ('\0') is unnecessary, but it is included for correctness as character arrays should be end-delimited by the NULL character.

By using malloc, you allocate (dynamic) memory for the buffer that we are copying the string to. Anytime memory is dynamically allocated, it should also be later freed otherwise, it will continue to be used unnecessarily. In order to free up this use of malloc, the 'free' command is used at the end, before the application returns.

Answer 3

as many told me to initialize the value of q to a memory address.I think there is no need to do this.because this code is working properly despite of undefined behavior.

 int main()
 {
 char *p="krishna";
 char *q,*r;
 r=q;

  while(*p!='\0')
  {
   *q++=*p++;
  }
   *q='\0';
    q=r;
  printf("%s",q);
  getch();
  return 0;
 }

Answer 4

You need to put a '\0' character on the end of q. You also need to allocate enough memory for it.

#include <string.h>
#include <stdlib.h>

int main()
{
  char *p="krishna";
  int size = strlen(p) + 1;
  char *q = (char *) malloc(size);
  char *qi = q;

  do
  {
   *q++=*p;
  } while(*p++!='\0');

  q = qi; //reset q to the beginning of the string

  printf("%s",q);
  getch();
  return 0;
}

I think this should work

Answer 5

a) You move q but don't reset it to the beginning of the string.

As many others have pointed out, you also need the destination of the copy operation be someplace where memory is allocated, on the heap with char *buf = malloc(BUF_SIZE) or on the stack with char buf[BUF_SIZE].

Finally the destination must be zero terminated.

b) Using a dedicated API is normally more readable and better performing that home grown loops.

Answer 6

One important improvement suggestion, which is missing yet (others have already pointed out the string related problems)

Your declaration of main is not correct. 
The return value should  not be  void  but int

Answer 7

void main()
{
  char *p="krishna";
  char *q;

  /* When copying, you modify your pointers, so you need another one
  ** to store where your string is starting. */
  char *r;

  /* Until now, q points to nowhere.
  ** You need to allocate the place it should point: */
  r = q = malloc(strlen(p) + 1);
  /* Plus 1 because C string uses one extra character in the end,
  ** the NULL ('\0') character. */

  while(*p!='\0')
  {
   *q++=*p++;
  }

  /* Until now, you've copied every character, except the NULL char at the end.
  ** you must set it in order for it to be a valid string. */
  *q = '\0';

  /* Now q points to the last (NULL) character of the string.
  ** In order to print it, you will need a pointer to the start of the string,
  ** that is why we need r: */
  printf("%s",r);

  /* When you are done using a memory buffer you allocated yourself,
  ** you must free it so it can be reused elsewhere. */
  free(r);

  getch();
}

Answer 8

You are moving the pointer at the end with for loop. Also, you should be allocating memory for q too.

Try this:

void main()
{
 char *p="krishna";
 char *q = malloc(sizeof(char) * 20);
 char *temp = NULL;

 /* save q */
 temp = q;
  while(*p!='\0')
  {
   *q++=*p++;
  }
  /* reset saved q */
  q=temp;
  printf("%s",q);
  getch();
}

Answer 9

You're not saying how much memory you want for q, so, you're trying to run over something else's memory. So, use malloc, or, arrays.

Answer 10

You should allocate memory for 'q' to hold the copied string.