Unix/C - 2 methods have different behaviour

Question

I am beginner at UNIX C development and need little bit of help. I have these two functions:

void edit_char(){

     int i;
     int length = strlen(expression);
     char *tmp = (char *) malloc((length+1)*sizeof(char));
     pom[0]='*';
     for(i=1;i<length+1;i++){
         tmp[i] = expression[i-1];
     }
     strcpy(expression,tmp);
     free((void *) tmp);
 }



 char *edit_char2(char *string){

     int i;
     int length = strlen(string);
     char *tmp = (char *) malloc((length+1)*sizeof(char));
     tmp[0]='/';
     for(i=1;i<length+1;i++){
         tmp[i] = string[i-1];
     }
     strcpy(string,tmp);
     free((void *) tmp);
     return string;
  }

edit_char() edits global variable char *expression - it puts a symbol "*" at the beginning. Second edit_char2() do almost the same, but instead of editing global variable, it edits string from argument.

First function works fine, the problem is with the tmp variable of second function. Malloc doesn't return empty char array with size of (length+1). It returns "xd\372\267xd\372\267\020".

What could cause this?

Answer 1

malloc just returns a pointer to the newly allocated memory, which is not to be considered "empty". If you want to "empty" it - i.e, fill your memoy with NULLs, you need to do it manually need to use calloc or fill it manually, for instance with:

• bzero (deprecated)
• memset

EDIT: Also I think that since you are adding a character to each string, you shouldn't use

newstring = malloc((strlen(string) + 1) * sizeof(char))

but instead

newstring = malloc((strlen(string) + 2) * sizeof(char))

To allocate space both for the new character AND the terminating \0 into account.

EDIT2: Which also means that your functions can't work / are not safe ! You are trying to make both string and expression contain 1 more character than they were probably initially allocated for !

Answer 2

 tmp[0]='/';
 for(i=1;i<length+1;i++){
     tmp[i] = string[i-1];
 }

You are not copying the trailing null character of string. This is needed for strcpy(string,tmp);

Note that you can use memmove to perform the copy instead of the for loop. Also note that the cast of the return value of malloc is not required and should be avoided. The cast of the free argument is also not required.

Answer 3

As stated before, your code won't work.

The error appears when using strcpy(target, tmp) in both functions. You need to understand that by doing this, you are almost certainly overflowing the memory pointed by target. If target just points to an array of strlen(target + 1) (all the characters in target plus a trailing NULL), then you are copying the content of tmp too a memory array that is one char too short. To illustrate this, running a loop such as :

/* allocate two strings, containing copies
   of constant string "Hello World" */
local_string = strdup("Hello World");
expression = strdup("Hello World");
for (i = 0; i < 100; i++) {
    edit_char();
    edit_char2(local_string);
    printf("global_string after: [%s]\n", expression);
    printf("local_string after: [%s]\n", local_string);
}

Will almost always lead to abnormal termination of the program, long before the 100th iteration. On Debian Linux Squeeze I get this output:

user@host$ ./a.out
global_string before: [Hello World]
local_string before: [Hello World]
global_string after: [*Hello World]
[...]
global_string after: [************Hello World]
local_string after: [////////////Hello World]
*** glibc detected *** ./a.out: double free or corruption (!prev): 0x00000000020e2050

You need to use a bit more pointer magic in order to achieve want you want. Here is working example, with a design improvement limiting code duplications:

Output

 user@host$ ./a.out
global_string before: [Hello World]
local_string before: [Hello World]
global_string after: [*Hello World]
local_string after: [/Hello World]

Code

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

char        *global_string;

void        add_one_leading_character(char leading, char **pointer_to_string)
{

    char    *new_string;
    char    *old_string;

    /* old_string will hold the content of
       pointer_to_string, for convenience */
    old_string = *pointer_to_string;
    /* allocate a new, empty string
       (= filled with NULLs) holding enough space
       for an additional character */
    new_string  = calloc(sizeof(*old_string), strlen(old_string) + 2);
    /* new_string now holds the leading character,
       followed by NULLs */
    new_string[0] = leading;
    /* concatenate the old_string to the new_string */
    strcat(new_string, old_string);
    /* make the pointer parameter points to the
       address of new_string */
    *pointer_to_string = new_string;
    /* free the memory pointed by old_string */
    free(old_string);
}

int        main(int ac, char **av)
{
    char   *local_string;

    /* allocate two strings, containing copies
       of constant string "Hello World" */
    local_string = strdup("Hello World");
    global_string = strdup("Hello World");
    printf("global_string before: [%s]\n", global_string);
    printf("local_string before: [%s]\n", local_string);
    /* add leading characters */
    add_one_leading_character('/', &local_string);
    add_one_leading_character('*', &global_string);
    printf("global_string after: [%s]\n", global_string);
    printf("local_string after: [%s]\n", local_string);
}