I am trying to make recursive function which is reading from file character wise but it's not working in C

Question

I have been getting hands on recursive technique using C , here the problem I am facing -

bool FetchInputFromFile(int file_dis ){
// file_dis is the file descriptor which I have used in main with `open` sys call
char ch;                            //I want to read char wise
size_t ch_size =sizeof(ch );
char temp[30];
size_t index =0;
size_t numread;

  //memset(temp, 0, 30 );
  numread =read(file_dis, &ch, ch_size );
    if(ch == ' ' ){
      temp[index ] = '\0';
      index =0;
      InsertInList(temp );                //calling function with temp 
   }else temp[index++] = ch;
     //1//
//base case and recursive call

if(numread ==0 ) return true;
else if(numread == -1 )return false;
else FetchInputFromFile(file_dis );

}

if I put printf("%s", temp ); where I have mentioned //1// above then the output is coming fine but if I call function over there , its going character wise.

What I am trying to do is I am reading file with open sys call and I am passing the file to the above function and trying to read char by char.But, it's not happening. Please help me how I can call function where the output goes word by word.

THANKS!!!!

There are many things wrong with your function. For example not all execution paths return a value; You seemingly don't know what [`read`](http://man7.org/linux/man-pages/man2/read.2.html) returns; And that all variables you have in the function are local to the current call and the current call only. — Some programmer dude, Mar 24 '17 at 12:26
@Someprogrammerdude Please let me know the correct form. And please do let me know in what the issue with local variables, and yeah I believe `index` could be the problem and I tried that making global but simply its not working. Coming to read part, well, if it's read successfully , it returns 0 , if not it returns -1, and there are some case where there is signal interruption but we can ignore that for this small function. `man read` what I have been following. thanks! — Devesh Pratap, Mar 24 '17 at 12:33
`read ` like you understood does not return the character read. So `if(numread == ' ' )` is not correct. The character read will be stored in ch. So you can check that. — Ajay Brahmakshatriya, Mar 24 '17 at 12:37
yes, correct, this is the problem in this code, but I was trying the condition with `ch` only. Thanks and I got the solution also. — Devesh Pratap, Mar 24 '17 at 12:45

score 0 · Answer 1 · answered Mar 24 '17 at 12:50

What I have trying to do is making temp and index local variables. hence, I changed my function call-

bool FetchInputFromFile(char * temp, size_t * value, int file_dis );

and I added one line inside the function, which is,

size_t index =*value;

and removed memset call.

Please post me if I can make this function call better.

Thanks for help.

score 0 · Accepted Answer · answered Mar 24 '17 at 13:16

I think this might be a better approach

bool FetchInputFromFile(int file_dis, char* nextWord, int* index)
{
#define MAXSTRINGLENGTH (30)

    /* file_dis is the file descriptor which I have used in main with `open` sys call */
    const size_t ch_size = sizeof(char);        /* size of types not variables.*/
    char currentChar;                           /* I want to read char wise*/
    size_t readResult;

    if (!nextWord)
    {   /* Allocate memory for the buffer if there is none */
        nextWord = (char*)malloc(sizeof(char) * MAXSTRINGLENGTH);
    }

    readResult = read(file_dis, &currentChar, ch_size);
    if (currentChar == ' ') 
    {
        nextWord[(*index)] = '\0';              /* Terminate the string*/
        InsertInList(nextWord);                 //calling function with temp */
        *index = 0;                             /* Reset the index to zero to reuse the buffer nextWord*/
    }
    else if ((*index) < MAXSTRINGLENGTH)
    {
        nextWord[(*index)++] = currentChar;
        //1*/
    }
    //base case and recursive call*/

    if (readResult == 0)
    {
        if (nextWord)           /* Should be a function/macro*/
        {
            free(nextWord);
            nextWord = NULL;
        }
        return true;
    }
    else if (readResult == -1)
    {
        if (nextWord)           /* Should be a function/macro*/
        {
            free(nextWord);
            nextWord = NULL;
        }
        return false;
    }
    else
    {
        return FetchInputFromFile(file_dis, nextWord, index);
    }
}

(I have not compiled this!)

Firstly when you write to the string you need to check that you don't over flow the buffer. You need to ensure every branch returns a value OR just have a single return statement and return a variable that the different branches set. And most importantly, format you code clearly, you write the code one, but it is read many more times, so take those few moments to add curly braces and tabs.

And I still think this is a bad way to read words, but I assume there is a reason to do it this way.

The code you have written is nice, Thanks for your time. And I need to tell you something, the code I wrote in my question is the Lazy code form. Like, you declared `#define MAXSTRINGLEN 30` this I have declared at the top of the code. But, I really want to know why you used `malloc` because what I am doing is I am calling my function from `main` and I have already allocated space in main for my `temp`. And, please let me know some other ways to read , what I am doing is reading very big file and I am storing each word in List. Please share your views it will help, THANKS!! — Devesh Pratap, Mar 24 '17 at 13:40
'Lazy code form' unless you say that it makes it look like you don't know what you are doing. People will waste time explaining things to you, that you knew, just couldn't be [bothered[ to write. Always write good code on here, it saves people time and you'll get better answers. Malloc, I used malloc because I didn't have a calling function. Use the static buffer in main, BUT you should pass the Max size of the buffer in as a parameter. — Code Gorilla, Mar 24 '17 at 14:54
You could read larger chunks of the file into a buffer and then process the buffer. This will be much more efficient and quicker, but you would end up with two recursive functions, one for the string parsing and one for loading from buffer to file. If you did that strtok would come into its own, yout just keep calling it and it will return each word for you as a NULL terminated string. This link will explain the official C version http://man7.org/linux/man-pages/man3/strtok.3.html, but this is clearer IMO http://www.cplusplus.com/reference/cstring/strtok/ — Code Gorilla, Mar 24 '17 at 14:57
You could do it with a 30 character buffer, but the bigger the buffer the better, usually. — Code Gorilla, Mar 25 '17 at 16:36
You are right, I was digging into that and I got the same answer, Thanks!! — Devesh Pratap, Mar 25 '17 at 17:52

I am trying to make recursive function which is reading from file character wise but it's not working in C

2 Answers2