0

I'm trying to write a macro for strcpy, but I'm not quite sure why the one I wrote doesn't work. Could it be that I am missing something important?

This is the one I have written:

#define _strcpy(line, save)                     \
        {                                       \
            while ((*save++ = *line++) != '\0') \
            ;                                   \
        }
jr.çhåvez__07
  • 170
  • 2
  • 9
  • 3
    Define "doesn't work". But in general, this macro is a recipe for a **lot of** troubles. – Eugene Sh. Mar 24 '21 at 15:54
  • 3
    When you call the macro, you change the variables involved: `_strcpy(temp, "foobar")` cannot work because you cannot do `"foobar"++`... also if it would "work" your `temp` would point past the end of the string....Oh, your macro has arguments reversed :-) – pmg Mar 24 '21 at 15:57
  • 3
    One problem: The macro will (try to) modify its arguments. If you really need your own implementation/replacement instead of an existing library function I suggest to use an `inline` function instead of a macro. – Bodo Mar 24 '21 at 15:57
  • `#define _strcpy(dest, src) strcpy((dest), (src))`? – Govind Parmar Mar 24 '21 at 15:58
  • 2
    Also, note that in the standard `strcpy` function, the first argument is the destination and the second is the source. – Adrian Mole Mar 24 '21 at 15:58
  • 1
    Some compilers might complain about the [unnecessary semicolon](https://stackoverflow.com/q/257418/1679849) after `_strcpy(from, to);`. This can be fixed by wrapping the macro with `do { ... } while (0)` – r3mainer Mar 24 '21 at 16:03

1 Answers1

4

You are indeed missing at least three important things.

Your _strcpy is taking arguments in the opposite order of a normal strcpy. This is extremely counter-intuitive and can lead to mistakes. You should define it as _strcpy(dst, src) instead (the destination is the first parameter).

Secondly, your macro directly modifies the arguments. This is ok in a normal function, but definitely not ok in the case of a macro! For example:

char *a = "hello";
char *b = malloc(6);

_strcpy(a, b);

// Now a and b are both invalid, pointing past the end of the strings.

To solve this, you should use temporary variables, and wrap your statements in a do { ... } while (0) block (see What's the use of do while(0) when we define a macro? for more information on why).

A decent final version of the macro would be:

#define _strcpy(dst, src)                \
        do {                             \
            const char *_src = (src);    \
            char *_dst = (dst);          \
                                         \
            while ((*_dst++ = *_src++))  \
                ;                        \
        } while (0)
Marco Bonelli
  • 63,369
  • 21
  • 118
  • 128