strcpy in C does not copy properly to overwrite a string

Question

I am trying to implement a infix to postfix conversion program in C. I wrote (cleanExpression) the following function to remove unwanted space in the given infix string expression.

#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
#include <assert.h>
#include <string.h>

#define EXPR_LIMIT 1000

bool isNumber(char c) {
    return (c >= '0' && c <= '9');
};

bool cleanInfixExpression(char *expr, char **clean) {
    *clean = malloc(sizeof(char) * strlen(expr) + 1);
    strcpy(*clean, expr);
    char *curr = *clean;
    char *temp;
    char *temp_dest;
    // first count the number of terms (numbers and operators).
    while (*curr != 0) {
        // check if the current position is space.
        if (*curr == ' ') {
            printf("Current:%s\n", curr);
            printf("Clean  :%s\n\n", *clean);

            // go through all space positions.
            for(temp = curr; *temp == ' '; temp++) {};

            // make its numbers on both side of the space.
            if (curr != *clean && (isNumber(*(curr-1)) && (isNumber(*temp)))) {
                printf("\"%s\" : Invalid expression!\n", expr);
                free(*clean);
                return (false);
            }

            // copy into the the original string.
            strcpy(curr, temp);
        }
        curr++;
    }
    printf("Current:%s\n", curr);
    printf("Clean  :%s\n\n", *clean);
    return (true);
};


void main(void) {
    char expr[EXPR_LIMIT] = " 214      +              9523        - 235235";
    printf("INFIX expression: %s\n", expr);
    char *clean;
    if (cleanInfixExpression(expr, &clean)) {
        printf("%s\n", clean);
    }
    return;
}

When I run this code the following is the output I get.

INFIX expression:  214      +            9523        - 235235
Current: 214      +            9523        - 235235
Clean  : 214      +            9523        - 235235

Current:      +            9523        - 235235
Clean  :214      +            9523        - 235235

Current:      952   9523        - 235235
Clean  :214+      952   9523        - 235235

Current:   9523        - 235235
Clean  :214+952   9523        - 235235

" 214      +            9523        - 235235" : Invalid expression!

However, if I change the strcpy(curr, temp) to the following code, where I manually copy, then I get the correct output.

temp_dest = curr;
while(*temp != 0) {
    *(temp_dest++) = *(temp++);
}
*temp_dest = 0;

This the output with me manually copying the string, instead of using the strcpy function.

INFIX expression:  214      +              9523        - 235235
Current: 214      +              9523        - 235235
Clean  : 214      +              9523        - 235235

Current:      +              9523        - 235235
Clean  :214      +              9523        - 235235

Current:              9523        - 235235
Clean  :214+              9523        - 235235

Current:        - 235235
Clean  :214+9523        - 235235

Current: 235235
Clean  :214+9523- 235235

Current:
Clean  :214+9523-235235

214+9523-235235

I am not able to understand what mistake I am making in using strcpy. Can someone tell me what might be going on?

I think the issue surrounds the fact you're using `strcpy(dst, src)` where `dst` and `src` are overlapping strings. The manual page for `strcpy` says that the strings may not overlap. The algorithm inside `strcpy` doesn't necessarily match the manual version you have. In addition, your manual copy changes `temp` but `strcpy(curr, temp)` does not. It's not clear what your context is for the manual version, but if you just put it directly in place of your `strcpy(curr, temp)` call, it's not doing the same thing. — lurker, Jan 04 '16 at 17:20
You should `malloc(sizeof(char) * (strlen(expr) + 1))` i.e. *strlen(expr) + 1 chars*, not *strlen(expr) chars + 1 byte*. — axiac, Jan 04 '16 at 17:26

score 2 · Accepted Answer · answered Jan 04 '16 at 17:26

You do not need to use malloc for cleanInfixExpression just to remove the spaces:

Do this

void cleanInfixExpression(char *expr)
{
   int r = 0;
   int w = 0;
   for (; expr[r]; r++) {
      if (expr[r] != ' ') {
         expr[w] = expr[r];
         ++w;
      }
   }
   expr[w] = 0;
 }

It will remove unwanted spaces from expr

score 1 · Answer 2 · answered Jan 04 '16 at 17:29

From the standard:

7.21.2.3 The strcpy function
Description
2 The strcpy function copies the string pointed to by s2 (including the terminating null character) into the array pointed to by s1. If copying takes place between objects that overlap, the behavior is undefined.

So you are not allowed to strcpy overlapping strings - as you do. You need to write your own implementation of strcpy or use memmove.

strcpy in C does not copy properly to overwrite a string

2 Answers2