1

I am having problems with my strcpy function. I am writing it without using the string.h header. It works, but, what if the dst is shorter than the src? How could I check it? Because would it be an error, if the dst would be example "car" and the src "green"?

#include <stdio.h>
#include <stdlib.h>

char* strcpy(char* dst, const char* src) {
    char* origDest =dst;

    while(*src != '\0') {
        *dst = *src;
        dst++;
        src++;
    }
    return origDest;
}

int main()
{
    char A[] = {"Blackcar"};
    char B[] = {"white"};
    char *C[10];

    *C = strcpy(&A, &B);

    printf("%s", *C);

    return 0;
}

Also, should I use malloc with this function?

Gilles 'SO- stop being evil'
  • 104,111
  • 38
  • 209
  • 254
BuffetBiffet
  • 49
  • 1
  • 4
  • may be you can check if `*dst != '\0'` in the `while` condition – Iharob Al Asimi Dec 13 '14 at 14:07
  • This is not a problem with your implementation; *in general*, you should not use `strcpy` to try and copy into a too small buffer. – Jongware Dec 13 '14 at 14:07
  • 1
    regarding also, should I use malloc with this function? if you want to mimic `strcpy` then no. – Iharob Al Asimi Dec 13 '14 at 14:07
  • just a suggention, change the `strcpy()` name to `my_strcpy()` to avoid any possible confusion later. good practice. – Sourav Ghosh Dec 13 '14 at 14:08
  • Just add a check like `if(strlen(B)>strlen(A))` before calling `strcpy`. Also change the name of your function as Sourav mentions so that you will not face errors once you include `string.h` in order to use `strlen` – Spikatrix Dec 13 '14 at 14:09
  • 1
    @CoolGuy not possible without string header. :-) – Sourav Ghosh Dec 13 '14 at 14:10
  • 1
    This shouldn't even compile. `char*` is not `char (*)[9]` or `char (*)[6]`. Maybe turn up some warnings in the future, or use a compiler without the dreadfully relaxed settings you're apparently using. – WhozCraig Dec 13 '14 at 14:11
  • @CoolGuy right you are, but I think, OP is _asked_ not to include the header. guessing a classroom assignment. :-) – Sourav Ghosh Dec 13 '14 at 14:15
  • 2
    @iharob: then it fails even if the destination buffer is large enough but *starts* with a `0`. There is no solution without passing the maximum allowed length as well. – Jongware Dec 13 '14 at 14:15
  • Right, I am really busy right now. Shouldn't comment wihtout thinking. – Iharob Al Asimi Dec 13 '14 at 14:27
  • create a function that calculates the length of a string, e.g. int len(char *p) { int n = 0; while (*p++) ++n; return n; } – AndersK Dec 13 '14 at 14:34
  • @SouravGhosh , In that case,OP will have to create a function that returns the length of the string passed to it similar to what CyberSpock has commented – Spikatrix Dec 13 '14 at 14:53
  • @CoolGuy right again. :-) – Sourav Ghosh Dec 13 '14 at 14:57
  • This isn't something you should (or could, within reason) worry about. Just as with the regular strcpy, it's the duty of the programmer to call it with proper parameters. – Etheryte Dec 13 '14 at 16:28
  • There's always the option of implementing a subset of the behavior of `strcpy_s` or `strncpy_s` (e.g. forget the runtime-constraint violation handlers and just return meaningful values) and giving it a different name. This fits your problem quite well, though it requires an additional parameter: the maximum number of chars to write (the destination buffer size). Additionally, `strncpy_s` is similar to `strncpy`, except extra bytes need not be written when the number of chars to read/copy is greater than the length of the source, and the destination is guaranteed to be null terminated. –  Dec 13 '14 at 17:42
  • @Jongware "This is not a problem with your implementation", OP's code does not even null charter terminate `dest`. – chux - Reinstate Monica Dec 14 '14 at 04:56
  • This code should give you compiler errors (aka. warnings), don't ignore them – M.M Dec 14 '14 at 05:26

3 Answers3

3

There is no way to check it in an elegant way because you cannot know the length of dst in your strcpy. Maybe in your main function, you can check length of string A with terminating \0, but no guarantee dst may not be garbage data or cleared with \0. That's one reason why strcpy can easily get a buffer overflow(Why should you use strncpy instead of strcpy?)

So my advice is add a length parameter in your function which means how many bytes will be copyed. Or maybe you could implement one version of strncpy yourself.

PS: Even with strncpy, the programmer have to make sure parameters passed to strncpy are valid and correct.

Community
  • 1
  • 1
D3Hunter
  • 1,329
  • 10
  • 21
  • On checking the length before calling: `char A[] = "1234\0\0\0\0";`, or simply `char A[10];`, would defeat that. – Jongware Dec 13 '14 at 16:27
  • 1
    You should *not* use `strncpy` instead of `strcpy`. It's not a "safer" version of `strncpy`. http://the-flat-trantor-society.blogspot.com/2012/03/no-strncpy-is-not-safer-strcpy.html – Keith Thompson Dec 14 '14 at 05:29
2

There is no way for the strcpy function to know the size of the destination buffer. This is why strcpy is somewhat deprecated: you can't tell, from looking at a call of the function, whether it's been used correctly or not.

If the destination buffer is not large enough, strcpy has undefined behavior: the rules of the language say that anything can happen. In practice, it'll overwrite some memory: a buffer overflow. It's up to the programmer to ensure that they only ever use strcpy safely. It's the programmer's responsibility, not the function implementer's responsibility, and in practice the implementer can't do anything about it: the size of the destination buffer is not available to check.

The destination buffer for strcpy doesn't need to contain a valid string. This is a perfectly valid use of strcpy:

char dst[10] = "car";
char src[12] = "green";
strcpy(dst, src);

The destination buffer is 10 bytes long, which is enough for a string of up to 9 characters (plus one for the terminating null byte). It happens to contain a shorter string at that moment, but that doesn't matter. Before the copy, dst is a 10-byte array containing something like

'c', 'a', 'r', 0, 'J', 'U', 'N', 'K', '.', '.'

(what's after the 0 byte can be anything, this is just an example). After the copy, dst contains

'g', 'r', 'e', 'e', 'n', 0, 'N', 'K', '.', '.'

To put it another way: strcpy doesn't exactly copy a string to a string. It copies a string to a buffer. After the copy, that buffer contains a string. What matters is that the destination buffer is large enough to accommodate the string.

strcpy writes to an existing buffer, it doesn't need to and shouldn't call malloc. There's a different function, strdup, which copies a string and allocates just enough memory for it; this function doesn't take the destination buffer as an argument (how could it: it creates the destination buffer).

Gilles 'SO- stop being evil'
  • 104,111
  • 38
  • 209
  • 254
  • Your example can possibly be improved by leaving the contents of `dst` *undefined*. As you point out later, its initial contents is totally unimportant for `strcpy`. – Jongware Dec 13 '14 at 16:25
  • 1
    @Jongware I purposefully put something in `dst` to show that it was being overwritten. I considered saying “it's undefined” but that's a bit subtle for someone who needs to ask this question. – Gilles 'SO- stop being evil' Dec 13 '14 at 16:29
1

This char* strcpy(char* dst, const char* src) never writes a '\0' to dest!
Suggest that is why OP said "having problems with my strcpy function".

True there is no way, without additional parameters passed to know the size of the destination, but at least insure dest is terminated correctly.

char* BB_strcpy(char* dst, const char* src) {
    char* origDest =dst;

    while(*src != '\0') {
        *dst = *src;
        dst++;
        src++;
    }
    *dst = 0;   // add this
    return origDest;
}

Standard strcpy() does not allocate memory. OP's version should not use malloc() either.

chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256