String modification through function. Unexplained behavior

Question

This Program should change n't followed by an alphabet to " not". like don't -> do not. But it's showing some really weird outputs. e.g if i just skip printing the string length output is garbage. also how space is changing to s2ace in output i can't say. If i didn't know better i'd say i have a ghost in the code.

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

int func(char **str, int cur_len, int max_len)
{
    if (cur_len <= 3)
        return(cur_len);
    else if(max_len < cur_len)
        return(-1);

    int i,j;
    char new[max_len];
    new[0]='\0';
    char temp[2];

    for (i = 0; i < cur_len; i++)
    {
        if(i > 0 && i < cur_len - 2 && isalpha((*str)[i-1]) && (*str)[i] == 'n' && (*str)[i+1] == '\'' && (*str)[i+2] == 't')
        {
            strcat(new," not");
            i=i+2;
        }
            else
        {
            temp[0]=(*str)[i];
            temp[1]='\0';
            strcat(new, temp);
        }
    }
    j = strlen(new);
    if (j > max_len)
        return(-1);

    *str = new;
    return(j);

}


int main()
{
    char *x = "Please don't do anything stupid. all the n't followed by an alphabate should be changed to not followed by a space. e.g. doesn't.";
    int len, max, result;
    len = strlen(x);
    printf("String size = %d\n",len);//**if i comment this line all output goes down the drain.**
    max = 200;

    result = func (&x, len, max);
    if(result == -1)
    {
        printf("No change possible\n");
        return(-1);
    }
    printf("No of changes = %d\n",strlen(x)-len);
    printf("New String is :: %s\n",x);//**how does space change into s2ace??**
    return(0);
}

Output is

129No of changes = 2 New String is :: Please do not do anything stupid. all the n't followed by an alphabate should be changed to not followed by a s2ace. e.g. does not.

You are returning a pointer to a local object from your function `func`. This has no defined behavior, anything can happen, so there is nothing to explain. — Jens Gustedt, Sep 22 '13 at 20:40
possible duplicate of [Returning local data from functions in C and C++ via pointer](http://stackoverflow.com/questions/3127507/returning-local-data-from-functions-in-c-and-c-via-pointer) — Jens Gustedt, Sep 22 '13 at 20:42
Yeah, you either need to `malloc()` the buffer in `func()`, or create an array in `main()` and pass that to your function. — Crowman, Sep 22 '13 at 21:11

score 0 · Answer 1 · answered Sep 22 '13 at 21:43

0

Fix the problem with minimal code changes by replacing char new[max_len]; with

char *new = *str = (char*)malloc(max_len+1);

You'll also need to add #include <stdlib.h> at the top.

As mentioned in comments, pointers to local objects (with auto storage class) become invalid as soon as the function returns.

Static objects won't disappear, but I don't believe that variable-length arrays are allowed to be static. So, you're stuck with malloc/calloc.

Be sure to document this behavior, so that the author of the calling function knows to free the pointer when the memory is no longer needed. (If you're going to quibble that you are the author and already know it, I'll point out that it helps the reader of your code to know what's going on.)

answered Sep 22 '13 at 21:43

Mike Housky

3,959
1
17
31

1

Probably don't want the `= *str` in there, the function needs to access the original value of the argument. Normal comments about casting the return from `malloc()` apply. – Crowman Sep 22 '13 at 21:47
@PaulGriffiths That's not the way I read the original program. The str argument is passed as char**, so the allocated string pointer is going to be returned. The name "new" referred to the array when it was incorrectly allocated as a C99 variable-length array, and is declared and allocated as an alias mostly to preserver old code. I'd declare a name for the pointer itself, anyway, and use that instead of *str. Even the name "new" works out to be descriptive. As I said, minimal changes, achieving intended behavior. – Mike Housky Sep 26 '13 at 20:54
I meant things like this: `temp[0]=(*str)[i];` and `isalpha((*str)[i-1]) && (*str)[i] == 'n'` which dereference the original double pointer passed in. If you overwrite that original value with `*str = anything` before getting there then that code is going to break. Obviously there's lots of other severe problems with this code, so who knows if that's what the OP actually intended to do, but the function certainly seems intended to read the original string passed in, and only then to modify what the original pointer points to. – Crowman Sep 26 '13 at 21:08

String modification through function. Unexplained behavior

1 Answers1