12

In an interview, I was asked to write an implementation of strcpy and then fix it so that it properly handles overlapping strings. My implementation is below and it is very naive. How do I fix it so that:

  1. It detects overlapping strings and
  2. after detecting, how do we deal with the overlap and proceed?

char* my_strcpy(char *a, char *b) { if (a == NULL || b == NULL) { return NULL; } if (a > b) { //we have an overlap? return NULL; } char *n = a; while (*b != '\0') { *a = *b; a++; b++; } *a = '\0'; return n; } int main(int argc, char *argv[]) { char str1[] = "wazzupdude"; char *after_cpy = my_strcpy(str1 + 2, str1); return 0; } 

EDIT:

So one possible implementation based on @Secure's answer is:

char* my_strcpy(char *a, char *b) { if (a == NULL || b == NULL) { return NULL; } memmove(a, b, strlen(b) + 1); return a; } 

If we don't rely on memmove, then

char* my_strcpy(char *a, char *b) { if (a == NULL || b == NULL) { return NULL; } if (a == b) { return a; } // case1: b is placed further in the memory if ( a <= b && a + strlen(a) > b ) { char *n = a; while(*b != '\0') { *a = *b; a++; b++; } *a = '\0'; return n; } // case 2: a is further in memory else if ( b <= a && b + strlen(b) > a ) { char *src = b + strlen(b) - 1; // src points to end of b char *dest = a; while(src != b) { *dest = *src; dest--; src--; // not sure about this.. } *a = '\0'; return a; } } 
11
  • 3
    How is a > b supposed to "detect an overlap"? It merely tests the two addresses. Commented Sep 15, 2011 at 8:08
  • You could do two copies: first copy to a local buffer, with no chance of overlap, then from the local buffer to the destination. Commented Sep 15, 2011 at 8:16
  • @pmg: you could, but then my_strcpy would have to be allowed to fail ENOMEM. Commented Sep 15, 2011 at 9:05
  • @Steve: right -- "There ain't no such thing as a free lunch"; although doing two copies is very far from a free lunch in the first place :-) Commented Sep 15, 2011 at 9:26
  • Regarding your edit, as an interviewer my very next question would be: Why would you not rely on memmove, and instead trade a one-liner against an unmaintainable pointer handling mess? Commented Sep 15, 2011 at 12:20

9 Answers 9

13

There is no portable way to detect this. You have to do pointer comparisons, and these are only defined within the same object. I.e. if the two strings do not overlap and are in fact different objects, then the pointer comparisons give you undefined behaviour.

I would let the standard library handle this, by using memmove(a, b, strlen(b) + 1).

EDIT:

As Steve Jessop pointed out in the comments, there actually is a portable but slow way to detect overlap in this case. Compare each address within b with the first and last address of a for equality. The equality comparison with == is always well defined.

So you have something like this:

l = strlen(b); isoverlap = 0; for (i = 0; i <= l; i++) { if ((b + i == a) || (b + i == a + l)) { isoverlap = 1; break; } } 

EDIT 2: Visualization of case 2

You have something like the following array and pointers:

S t r i n g 0 _ _ _ _ _ _ _ ^ ^ | | b a 

Note that b + strlen(b) results in a pointer to the terminating \0. Start one behind, else you need extra handling of edge cases. It is valid to set the pointers there, you just can't dereference them.

src = b + strlen(b) + 1; dst = a + strlen(b) + 1; S t r i n g 0 _ _ _ _ _ _ _ ^ ^ ^ ^ | | | | b a src dst 

Now the copy loop which copies the \0, too.

while (src > b) { src--; dst--; *dst = *src; } 

The first step gives this:

src--; dst--; S t r i n g 0 _ _ _ _ _ _ _ ^ ^ ^ ^ | | | | b a src dst *dst = *src; S t r i n g 0 _ _ _ 0 _ _ _ ^ ^ ^ ^ | | | | b a src dst 

And so on, until src ends up equal to b:

S t r i S t r i n g 0 _ _ _ ^ ^ | | b a src dst 

If you want it a bit more hackish, you could compress it further, but I don't recommend this:

while (src > b) *(--dst) = *(--src); 
Sign up to request clarification or add additional context in comments.

16 Comments

It's not true that overlap can't be detected portably. There is a shockingly inefficent way. This is for memmove, but I believe it can be adapted for strcpy: stackoverflow.com/questions/4023320/…
"You have to do pointer comparisons, and these are only defined within the same object." -How do I check if the two pointers point to the same object (an array in this case)
@Steve Jessop: Correct, but in this case it can't be detected portably, because you only have two pointers, but not the array boundaries (start address and size) they're pointing into.
@user639309: It must be given as a known precondition. Else you can't check for it without invoking undefined behaviour if they're not, except when the array boundaries are known as Steve pointed out. Then you can check each possible address inside one of the arrays to be equal to the other pointer. OTOH, if you know the boundaries for both arrays, you can simply check them for equality. An equality check of pointers with == is always well defined, the same-object rule applies only for less than or greater than checks.
@Secure: But as you've said yourself, strlen(b)+1 gives you that size. It only goes wrong if the caller has done something invalid first, e.g. if a doesn't point to a big enough buffer, but that's not our fault.
|
4

You could probably use memmove() if you expect the strings to be overlapping.

char* my_strcpy(char *a, char *b) { memmove(a, b, strlen(b) + 1); return a; } 

4 Comments

That's taking into account that one char == one byte. I'd change strlen(b) + 1 by ( strlen(b) + 1 ) * sizeof( char )
sizeof(char) is always exactly one byte.
Yep, but memmove expects bytes, not chars, even if they incidentally have the same size. Anyway, I just said "I would".
@Baltasarq "memmove expects bytes, not chars" is misleading. memmove() expects a size in characters and in C, "bytes" and character have the same size. "The memmove function copies n characters from the object pointed to by s2 into the object pointed to by s1. C11 "7.24.2.2"
4

Note: Here, b is the address of the source string and a is the address of the destination.

With a > b you wouldn't necessarily have an overlap. If

(a <= b && a+strlen(a) >= b) || (b <= a && b+strlen(b) >= a) 

then you have an overlap.

However, besides detecting overlap for the sake of interview, a > b should do fine for strcpy. The idea is this:

If b is placed further in the memory (b > a), then you can normally copy b into a. Parts of b will be overwritten, but you are already past that part.

If a is placed further in the memory (a > b), it means that possibly by writing on the first location of a, you have already overwritten a location in b with a higher index. In such a case, you should copy in the opposite direction. So instead of copy from index 0 to strlen(b)-1, you should copy from strlen(b)-1 to 0.

If you are confused how that helps, draw two overlapping arrays on paper and try to copy once from the beginning of the array and once from the end. Try this with the overlapping arrays both in cases a > b and a < b.

Note, if a == b, you don't need to actually copy anything and you can just return.

Edit: I am not sure, but reading the other solutions, it seems like this answer may not be completely portable. Beware of that.

2 Comments

if a==b, you could even just return :-) strcpy takes pointers-to-non-volatile, so there's no requirement to actually touch the memory. That said, it's not worth adding code to optimize that absurd case.
@chux, did you take the terminating NUL into account?
3
if a > b; then copy a from the beginning else if a < b; then copy a from the ending else // a == b do nothing 

You can refer to an implementation of memmove, it's quite like what I said.

1 Comment

a > b risks UB when a and b are in different objects - so not a portable solution.
2

Even without using relational pointer comparisons, memmove, or equivalent, it is possible to code a version of strcpy which will be performed as an strlen and memcpy in the non-overlapping case, and as a top-down copy in the overlapping case. The key is to exploit the fact that if the first byte of the destination is read and then replaced with zero, calling strlen on the source and adding to the source pointer the value that was returned will yield a legitimate pointer which will equal the start of the destination in the "troublesome overlap" case. If the source and destination are different objects, the "source plus strlen" pointer may be safely computed and observed to be unequal to the destination.

In the event that adding the string length to the source pointer yields the destination pointer, replacing the zero byte with the earlier-read value and calling strlen on the destination will allow code to determine the ending address of the source and destination strings. Further, the length of the source string will indicate the distance between the pointers. If this value is large (probably greater than 16 or so), code may efficiently subdivide the "move" operation into a top-down sequence of memcpy operations. Otherwise the string may be copied with a top-down loop of single-byte copy operations, or with a sequence of "memcpy to source to buffer"/"memcpy buffer to destination" operations [if the per-byte cost of a large memcpy is less than half that of an individual-character-copy loop, using a ~256-byte buffer may be a useful optimization].

1 Comment

"first byte of the destination is read and then replaced with zero," --> Interesting.
1
if (a>= b && a <= b+strlen(b))) || (b+strlen(b) >= a && b+strlen(b) <= a + strlen(b)) 

(*) you should cache strlen(b) to improve performance

What it does:
checks if the a+len [address of a + extra len bytes] is inside the string, or a [address of a] is inside the string, these are the only possibilities for a string overlapping.

1 Comment

a>= b risks UB when a and b are in different objects - so not a portable solution.
1

If these two strings overlap, then, while copying you'll run over the original a or b pointers.

Assuming that strcpy( a, b ) roughly means a <- b, i.e., the first parameter is the destination of the copy, then you only check whether the copy pointer reaches b's position.

You only need to save the b original position, and while copying, check you haven't reached it. Also, don't write the trailing zero if you have reached that position.

 char* my_strcpy(char *a, const char *b) { if ( a == NULL || b == NULL ) { return NULL; } char *n = a; const char * oldB = b; while( *b != '\0' && a != oldB ) { *a = *b; a++; b++; } if ( a != oldB ) { *a = '\0'; } return n; } 

This algorithm just stops copying. Maybe you want to do something else, such as marking the error condition, or add an end-of-the string mark to the previous position (though failing silently (as the algorithm does at the moment) isn't the best option).

Hope this helps.

Comments

1

I was asked this in a recent interview. We don't have to 'detect' overlap. We can write strcpy in such a way that overlapping addresses are taken care of. The key is to copy from the end of source string instead of from the start.

Here is a quick code.

void str_copy(const char *src, char *dst) { /* error checks */ int i = strlen(a); /* may have to account for null character */ while(i >= 0) { dst[i] = src[i]; i--; } } 

EDIT: This only works when a < b. For a > b, copy from the start.

2 Comments

The problem persists in case of overlapping strings. Like memcpy, you should copy from beginning or the end, based on if the destination to copy has lower or higher address than the source respectively.
1) Code does not compile. 2) Suggest re-working answer/code to use src dest throughout rather than a b. 3) strlen() returns type size_t, but then size_t i causes trouble with while(i>=0) test which is always true.
0

This SO entry is already older, but I am currently working on a old piece of code that copies overlapping strings with strcpy(). Characters were missing in the log output. I decided to use the following compact solution, which copies char by char.

static char *overlapped_strcpy(char *dest, const char *src) { char *dst = dest; if (dest == NULL || src == NULL || dest == src) return dest; do { *dst++ = *src; } while (*src++); return dest; } 

EDIT:

As @Gerhardh pointed out, the code above only works if dest <= src (I just had to solve this case). For the case dest > src it is more complicated. However, copying from behind, as other answers have already mentioned, leads to success. For example:

if (dest <= src) { /* do the above */ } else { int i = (int)strlen(src); while (i >= 0) { dst[i] = src[i]; i--; } } 

4 Comments

How does this address the overlapping part? Let's assume strlen(src) == 20 and dest=src+5
@Gerhardh I only had to solve strcpy(posPtr, posPtr+2);. The opposite case would be a mess.
Detecting and handling the mess would be the whole point of such a function. ;)
dest <= src risks UB when dest and src are in different objects - so not a portable solution.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.