CODEWITHSUNDEEP
c
Angus
Angus

Reputation: 12621

my_strcpy gives seg fault

#include<stdio.h>

char* my_strcpy(char*,const char*);

int main(){
        char a[20];
        char* s = "Hello world!";
        char* d = a;
        my_strcpy(d,s);
        printf("\n d : %s \n",d);
        return 0;
}

char* my_strcpy(char* dest,const char* sour){
        if(NULL == dest || NULL == sour){
                return NULL;
        }
        while(1){
                *dest++ = *sour++;
                if(*sour == '\0'){
                        *dest = *sour;
                        break;
                }
        }
}

why do we need the char* as a return type for my_strcpy. If the d is " " it gives me a segmentation fault. If I assign it with a it works fine. Why does it give seg fault when given "".

MODIFIED : After the answers

#include<stdio.h>

char* my_strcpy(char*,const char*);

int main(){
        char* ret;
        char a[20];
        char* s = "Hello world!";
        char* d = "";
        ret = my_strcpy(d,s);
        if(NULL == ret){
                perror("\nret");
        }
//      printf("\n d : %s \n",d);
        return 0;
}

char* my_strcpy(char* dest,const char* sour){
        char* temp;

        if(NULL == dest || NULL == sour){
                return NULL; 
        }

        temp = dest;
        while(1){ 
                *temp++ = *sour++;
                if(*sour == '\0'){
                        *temp = *sour;
                        break;
                }
        }
        return temp;
}

This still gives a segfault. How to handle the condition if s="" when passed to the function strcpy.

Upvotes: 0

Views: 798

Answers (5)

Lundin
Lundin

Reputation: 213970

I would advise to implement strcpy as:

char* my_strcpy (char* s1, const char* s2)
{
  char* return_val = s1;

  *s1 = *s2;

  while(*s2 != '\0')
  {
    s1++;
    s2++;
    *s1 = *s2;
  }

  return return_val;
}

Always avoid multiple ++ statements in the same expression. There is never a reason to do so and it will sooner or later give you a handful of undefined/unspecified behavior bugs.

Upvotes: 0

Tarik
Tarik

Reputation: 11209

You asked: "If the d is " " it gives me a segmentation fault." Answer: If you assign "" or " " to d, there will not be enough space to fit "Hello World". Moreover, a constant string if assigned to a memory page tagged as data might not allow modification.

You asked "why do we need the char* as a return type for my_strcpy" as the original strcpy I presume. Answer: You do not have to. You could have void as return type. However, it makes it practical if one is to do something like this:

printf ("%s", strcpy (dest, sour));

Corrected code:

    while(1){
            *dest++ = *sour++;
            if(*(sour-1) == '\0'){
                    break;
            }

or better:

            while(*sour != '\0'){
                *dest++ = *sour++;
            }
           *dest = *sour;

Upvotes: 3

technosaurus
technosaurus

Reputation: 7802

here is a simple version:

char *my_strcpy(char *d, const char *s){
  int i=0;
  while(d[i++]=*s++)
    /*done inside condition*/;
  return d;
}

Upvotes: 0

Grijesh Chauhan
Grijesh Chauhan

Reputation: 58271

Correct: 

 if(*dest == '\0'){

should be: 

 if(*(dest - 1) == '\0'){

Note:

 *dest++ = *sour++;

is equivalent to 

*dest = *sour;
sour++;
dest++;

You increments dest after assignment, and so you are checking \0 at position where garbage value present as you don't initialize a[] -causes Undefined behaviour. Additionally you don't return after while loop.

You can simply write your function as: 

char* my_strcpy(char* dest,const char* sour){
     if(NULL == dest || NULL == sour)
         return NULL;
    char* d = dest;
     while(*dest++ = *sour++)
        ;
     return d;
}

Give it a try!!

Upvotes: 2

Joni
Joni

Reputation: 111289

Here you:

  1. Assign the value at *sour to *dest
  2. Increment both sour and dest so now the point to the next characters
  3. Test if *dest is now NUL to exit the loop

As you can see, the 3rd step reads from uninitialized memory. You should test if the value that was assigned before incrementing was NUL.

            *dest++ = *sour++;
            if(*dest == '\0'){
                    break;
            }

If the dest you pass in is a constant string like " " you get a segfault because string constants are stored in read-only memory. They cannot be modified.

Upvotes: 2

Related Questions