2

I'm writing in C and have to return a char* I am trying to replicate the strcpy function. I have the following code

int main() 
{
    char tmp[100];
    char* cpyString;
    const char* cPtr = &tmp[0];

    printf("Enter word:");  
    fflush(stdin);
    scanf("%s", &tmp);

    cpyString = strcpy("Sample", cPtr);

    printf("new count is %d\n", strlen(cpyString));

}

int strlen(char* s)
{
   int count = 0; 

   while(*(s) != 0x00) 
   {
      count++;
      s = s+0x01;
   }
   return count;
}

char* strcpy(char* dest, const char* src)
{
    char* retPtr = dest;

    int i =0;
    int srcLength = strlen(src);

    for(i = 0; i< srcLength; i++)
    {           
       *(dest) = *(src); //at this line program breaks
        dest = dest + 0x01;
        src = src + 0x01;
    }

    *(dest) = 0x00; //finish with terminating null byte

     return retPtr;
}

Q1: How can I assign the dereferenced value at the src to the destination without the program crashing?

Q2: If I need to copy the tmp string entered into a new string, how would I do that? I can't seem pass tmp as the second parameter

MasterGuapo
  • 29
  • 1
  • 3
  • 1
    Cannot reproduce the error. Please include your compiler and show how you use `strcopy`. – Zeta Mar 21 '18 at 05:52
  • We need to see the code that calls your `strcopy` function and what argument value it assigns to the `dest` parameter. – Dai Mar 21 '18 at 05:53
  • Yes, I deleted it. The issue will be storage for `dest`. – David C. Rankin Mar 21 '18 at 05:54
  • 2
    You don't need to call `strlen` - you can simply check for `*src == '\0'` and stop the loop. – Dai Mar 21 '18 at 05:54
  • 4
    Please provide [**A Minimal, Complete, and Verifiable Example (MCVE)**](http://stackoverflow.com/help/mcve). I suspect your `dest` may be an uninitialized pointer in the caller. – David C. Rankin Mar 21 '18 at 05:55
  • Also, `i` and `srcLength` should be `size_t`, not `int` for a proper implementation. – Patrick Roberts Mar 21 '18 at 05:55
  • Using 0x01 when you mean 1 does not make you a wizard. Rather the reverse. – rici Mar 21 '18 at 06:01
  • Note that `const char* cPtr = &tmp[0];` is just `const char* cPtr = tmp;`, read your pointers chapter again if you're confused. Also, according to the standard `fflush(stdin)` is undefined behavior, some platforms imeplement the behavior for `stdin` only, but you should not rely on that, `fflush()` is only for output streams. And `s = s+0x01;`??? really??? Please also note, that even though you implemented `strlen()` yourself, you still used it when you could do a loop just like the one you did in `strlen()` to traverse through the characters of the string. – Iharob Al Asimi Mar 21 '18 at 06:09
  • 1
    And finally, "*at this line program breaks*" is not a helpful comment, please describe "*breaks*" precisely. – Iharob Al Asimi Mar 21 '18 at 06:10
  • Compile with all warnings and debug info (`gcc -Wall -Wextra -g` with [GCC](http://gcc.gnu.org/)). Improve the code to get no warnings. [use the debugger, e.g. `gdb`](https://sourceware.org/gdb/current/onlinedocs/gdb/). And avoid naming your own functions like standard ones (so use `mystrcpy` not `strcpy`) – Basile Starynkevitch Mar 21 '18 at 06:13

4 Answers4

7

Here

cpyString = strcpy("Sample", cPtr);
                   ^^^^^^^
                   const

you have swapped the arguments. The first argument is a string literal ("sample") that you are not allowed to write to. See https://stackoverflow.com/a/4493156/4386427

Try

cpyString = strcpy(cPtr, "Sample");

I'm not sure that the second line is exactly what you want but at least it is legal.

Maybe you really want:

cpyStringBuffer[100];
cpyString = strcpy(cpyStringBuffer, cPtr);

In general your code in main is more complicated than needed.

Try:

int main() 
{
    char input[100] = {0};
    char dest[100];

    printf("Enter word:");  
    scanf("%99s", input);     // notice the 99 to avoid buffer overflow

    strcpy(dest, input);

    printf("new count is %d\n", strlen(dest));

    return 0;
}
Support Ukraine
  • 42,271
  • 4
  • 38
  • 63
1

I guess you might have wanted to code like below.

#include <stdio.h>

int strlen(char* s);
char* strcpy(char* dest, char* src);

int main() 
{
    char tmp[100];
    char cpyString[100];

    printf("Enter word:");  
    fflush(stdin);
    scanf("%s", &tmp);

    strcpy(cpyString, tmp);

    printf("new count is %d\n", strlen(cpyString));

}

int strlen(char* s)
{
   int count = 0; 

   while(*(s) != 0x00) 
   {
      count++;
      s = s+0x01;
   }
   return count;
}

char* strcpy(char* dest, char* src)
{
    char* retPtr = dest;

    int i =0;
    int srcLength = strlen(src);

    for(i = 0; i< srcLength; i++)
    {           
       *(dest) = *(src); //at this line program breaks
        dest = dest + 0x01;
        src = src + 0x01;
    }

    *(dest) = 0x00; //finish with terminating null byte

     return retPtr;
}
  1. When you invoke your strcpy() in the main function, arguments src and dest are reversed.
  2. if you want to use the variable cpyString, then you are supposed to determine which to allocate pieces of memory from either static or dynamic.
    • In my example, I declared cpyString as an array of characters. Which means that the variable will occupy static memory partly.
    • You can also alternatively allocate bytes of dynamic memory to it by calling malloc() or calloc() function.
woniok
  • 159
  • 4
  • 9
0

I think you used non initialized destination and literal string pointer. You have to declare your destination as a buffer like

char dest[const_size]

So

char* strcpy(char* dest, const char* src)
{
char* retPtr = dest;

int i =0;
int srcLength = strlen(src);

for(i = 0; i< srcLength; i++)
{
   *(dest) = *(src); //at this line program breaks
    dest = dest + 0x01;
    src = src + 0x01;
}

*(dest) = 0x00; //finish with terminating null byte

 return retPtr;
}



int main()
{
 char *arr="xxxxxx";

char *dest="fffff";  // this won't work because you can not modify const string
char *dest_1;   // this won't work because it is uninitialized pointer
char dest_2[50]; // this will work fine 
strcpy(x, y);


 printf("%s",x);
     //x still the same as point pointer

return 0;
}
IDEN
  • 21
  • 1
  • 9
-1

Your program is crashing because you cant modify the pointer to a constant. Please find the corrected code below:

char *
mstrcpy (char *dest, const char *src)
{
  char *retPtr = dest;

  int i = 0;
  int srcLength = strlen (src);

  for (i = 0; i < srcLength; i++)
    {
      *(dest) = *(src);     //now doesn't break at this line
      dest = dest + 1;
      src = src + 1;
    }

  *(dest) = 0x00;       //finish with terminating null byte

  return retPtr;
}


int
main ()
{

  //char a = "abc";    // will cause crash
  char a[] = "abc";    // won't crash
  char *b = "xyz";

  mstrcpy(a,b); //works fine !!!!
  return 0;

}

Note that in the main function if you use char a = "abc" , then it will cause problem because its a pointer to a constant

Syed Waris
  • 1,056
  • 1
  • 11
  • 16
  • 1
    There's no need to call `strlen` when writing `strcpy`... you are actually making it less efficient (you iterate twice over the source string for no good reason). In general, a common implementation of `strcpy` has one third of the lines of yours (and a compact one is a three-liner). – Matteo Italia Mar 21 '18 at 06:09