CODEWITHSUNDEEP
c
AbhishekSaha
AbhishekSaha

Reputation: 745

char array wont print in C unless previously printed

So this is strange- below is my code to create a tokenizer object and the main method:

struct TokenizerT_ {
    char * sep;
    char * toks;
};

TokenizerT *TKCreate(char *separators, char *ts) {

    if (ts==NULL) {
        return NULL;
    }

    int lin = (int) strlen(separators);
    char yr[lin];
    yr[0] = *separators;
    int lim = 1;
    int h = 1; 

    for(h=1; h<strlen(separators); h++){
        char tmp = *(separators+h);
        int z=0;

        for (z=0; z<lim; z++) {
            if (tmp==yr[z]) {
                z=-1;
                break;
            }
        }
        if(z>-1){
            yr[h] = tmp;
            lim++;
        }

    }
    TokenizerT inu = {yr, ts};
    printf("%s\n", inu.sep);
    return &inu;
}

int main(int argc, char **argv) {

    char * arr = argv[1];
    char * y = argv[2];
    TokenizerT jer = *TKCreate(arr, y);

    printf("%s\n", jer.sep);
    printf("%s\n", jer.toks);
    return 0;
}

If I run the program with arguments "tes" and "testing", I get the following results: tes tes testing

However, if I run the program with the printf statement in TKCreate commented out, I get:

testing.

The printf("%s\n", jer.sep) stops working and I've done nothing to the code except commenting out that printf statement in TKCreate. Why is that happening?

Upvotes: 0

Views: 110

Answers (3)

Kevin
Kevin

Reputation: 2182

You need to allocate memory for an object that is to be returned from a function. This puts the object on the heap (permanent memory) vice on the stack (memory that is recycled by the program as soon as the current function ends).

You can do that by making these changes in your TKCreate function:

TokenizerT *TKCreate(char *separators, char *ts) {
    ...
    # change 'char yr[lin];' to:
    char* yr = (char*)malloc(lin * sizeof(char));

    ....
    TokenizerT* inu = (TokenizerT*)malloc(sizeof(TokenizerT));
    inu->sep = yr;
    int->yoks = ts;
    printf("%s\n", inu->sep);
    return inu;
}

You can also clean up your main() function by only sharing one copy of the object. You could call free() on any allocate memory once you are done with it:

int main(int argc, char **argv) {
    char * arr = argv[1];
    char * y = argv[2];
    TokenizerT jer* = TKCreate(arr, y);

    printf("%s\n", jer->sep);
    printf("%s\n", jer->toks);
    free(jer);
    return 0;
}

Upvotes: 0

David Heffernan
David Heffernan

Reputation: 613531

There are a number of problems here:

  1. inu is a local variable. You return the address of it. When you attempt to read from that address in main, you are accessing a variable that is no longer in scope. That is undefined behaviour.
  2. The sep field of inu is assigned yr. Now, yr is a local variable, a character array, which decays to a pointer. Again, accessing that once the function has returned means accessing a variable whose scope has ended. Again, undefined behaviour.
  3. You are using a variable length array, a VLA. I suspect that is not intentional. Generally speaking, VLAs are advanced features that should only be used when you are clear on the implications.

I suggest the following changes:

  • Allocate the character arrays dynamically using malloc.
  • Return the struct by value.

On top of that I suspect that you have other problems. It appears to me that yr is not null-terminated, and some elements may not be initialized at all. Is that intentional? Perhaps you actually meant to initialise yr to be equal to separators when you wrote yr[0] = *separators. Note that your code simply assigns a single character. To allocate yr dynamically, and initialise it to be equal to separators you can write:

yr := malloc(strlen(separators)+1);
strcpy(yr, separators);

Why do you call strlen(separators) multiple times? You appear to call it 1+strlen(separators) times. You should call it once only.

Upvotes: 3

Zan Lynx
Zan Lynx

Reputation: 54373

In C you can never return a pointer to a local function variable. Well, you can, but it won't work. Even if it seems to work, it is only waiting for a good opportunity to not work.

So you absolutely cannot do this:

TokenizerT inu = {yr, ts};
return &inu;

What you need to do is either pass in a pointer to the object you want to build, or use malloc to allocate a new one. If you use malloc you will need to remember to free the object later. And because of Windows and its multiple C runtimes it is a very good idea to free your objects inside a function just for that. Because if you were to have the code in a library, and malloc in the Debug runtime and free in the Release runtime, bad things happen.

So for the malloc example:

struct box* create_box(int x, int y) {
    struct box* b = malloc(sizeof(*b));
    if(!b) abort();
    b->x = x;
    b->y = y;
    return b;
}

void destroy_box(struct box* x) {
    free(x);
}

And for passing an object in:

void init_box(struct box* b, int x, int y) {
    b->x = x;
    b->y = y;
}

void f() {
    struct box b;
    init_box(&b, 10, 20);
}

Upvotes: 2

Related Questions