Getting characters and converting them into string

Question

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

static int cc =0;
static FILE* in;

static char* getkw(){
    char tok[10];
    int i;
    cc = fgetc(in);
    for(i=0; i<=(int)sizeof(tok)-1 &&isalpha(cc);++i){
        tok[i]= cc;
        cc = fgetc(in);
    }                    
    tok[i]=0;
    return tok;
}

int main()
{
    in = fopen("/dir/dir/a.txt", "r");
    char *c= getkw();
    printf("%s", c);
    fclose(in);
    return 0;
}

I've expected above code to get characters till space is reached. Also I have functions that ignore spaces from file and continue to next character which I've not shohwn here because they are (in my view) working well. The main problem here is, it doesn't get a character from file and doesn't pack them into array. The program crashes or nothing appears (if success).

Your indentation was weird. Did you like how your code looked? Or did you mix tabs and blanks, that easily messes up the indentation. — Yunnosch
– Yunnosch, Commented Jun 2, 2018 at 18:08
The problem with tok is that you allocate it on the stack, and when the function returns, you can't rely on its contents anymore. One way is to allocate tok dynamically, as @purec hints at. — Arndt Jonasson
– Arndt Jonasson, Commented Jun 2, 2018 at 18:08
Why is cc a global variable? Why isn't it local to the getkw() function? If you did ungetc(c, fp); you'd be able to reread the character after the keyword. As it is, you just throw that character away because you set cc unconditionally when you call getkw() again. — Jonathan Leffler
– Jonathan Leffler, Commented Jun 2, 2018 at 18:20

Weather Vane · Accepted Answer · 2018-06-02 18:41:08Z

Returning a pointer to a local variable will not work, because the array is no longer in scope. It is tok which should be static not the function.

There is another bug following

for(i=0; i<=(int)sizeof(tok)-1 &&isalpha(cc);++i)

which would be clearer if the condition is written as the more idiomatic

i<(int)sizeof(tok)

which is less work for the brain. Now it is clear that

tok[i]=0;

after the loop, when i == sizeof tok, is writing beyond the array bounds.

Edit: more generally about loop control.

There is another danger with your -1 way of doing things. Suppose I use your way in

size_t len = strlen(tok);
for(size_t i = 0; i <= len - 1; i++) { /*...*/ }

What will happen when len == 0? Disaster, because size_t is unsigned, and so len - 1 will wrap to the max value for the type, and there will be a huge loop.

Now, if you write the loop in the idiomatic way

size_t len = strlen(tok);
for(size_t i = 0; i < len; i++) { /*...*/ }

this cannot happen.

samuelnj · Accepted Answer · 2018-06-02 18:16:55Z

1

The problem is that in the function getkw() the array tok is a local variable, so the memory is freed when the function returns. Thus, the pointer you're trying to return becomes invalid and the program crashes when you try to print it.

To solve the problem you will either have to dynamically allocate memory with malloc and return the pointer, or pass a pointer to the array into the function and modify it.

For dynamic memory allocation:

char* tok = malloc(10);

For passing a pointer to array:

static void getkw(char[] * array){
    char[] tok = * array;
    ...
    //no need to return anything, since you modify the passed array
}

int main()
{
    char tok[10];
    getkw(&tok);
    ...
}

edited Jun 2, 2018 at 18:16

answered Jun 2, 2018 at 18:08

samuelnj

1,6471 gold badge14 silver badges20 bronze badges

Collectives™ on Stack Overflow

Getting characters and converting them into string

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related