Making a minishell in C. How to improve error control with feoh and ferror?

I've written this minishell but I'm not sure I'm making a correct control of the errors. Could you give me some advice?

#include 
#include  
#include  
#include 

#define LINE_LEN  50
#define MAX_PARTS  50

int main ()
{
char* token;
char str[LINE_LEN];
char* arr[MAX_PARTS];
int i,j;
bool go_on = true;

while (go_on == true){
    printf("Write a line:('quit' to end) \n $:");
    fgets(str, LINE_LEN, stdin);

    if (str==NULL) {
        goto errorfgets;
    } else {
        size_t l=strlen(str);
        if(l && str[l-1]=='\n')
            str[l-1]=0;

        i=0;
        /* split string into words*/
        token = strtok(str, " \t\r\n");
        while( token != NULL ) 
        {
            arr[i] = token;
            i++;
            token = strtok(NULL," \t\r\n");
        }

        fflush(stdin);

        /* check if the first word is quit*/
        if (strcmp(arr[0],"quit")==0)
        {
            printf("Goodbye\n");
            go_on = false;
        } else {

            for (j=0; j < i; j++){
            printf("pajaritos por aqui'%s'\n", arr[j]);     
            }   
        }
    }
}

return 0;
errorfgets:
    printf("fgets didn't work correctly");
    return -1;
}

I think I haven't really understood the point of your program but I'll try to give you a few hints :

• The indentation is not really good. I am not sure if this is because of SE but I'd rather point it out just in case.

• You'll hear everything and its opposite about goto. It surely has drawbacks but it can be quite useful to handle errors and free the ressources that are not required anymore. However, in your case, there's no need for goto so it's probably better to Keep It Simple and just use return.

• I am not a big fan of a go_on variable if break can do the trick. Also, it makes stdbool pointless.

• Things will be easier to understand if you declare local variables in the smallest possible scope. Depending on the version of C you are using, you can even declare j in your for statement : for (int j=0; j






• Because of return/break, you can now remove levels of indentation (again, this is purely personal preference).






• You shouldn't repeat "quit" in multiple places. It will make it hard to change if it has to change in the future.






• There is a risk that arr[i] = token;` goes to far if you ever change a constant without changing the other. Let's make sure it does not go too far.

At the end, here what the code would be like :

#include 
#include 
#include 

#define LINE_LEN  50
#define MAX_PARTS  50

static const char quit[] = "quit";

int main ()
{
    char str[LINE_LEN];
    char* arr[MAX_PARTS];

    for (;;) {
        printf("Write a line:('%s' to end) \n $:", quit);
        fgets(str, LINE_LEN, stdin);

        if (str==NULL) {
            printf("fgets didn't work correctly");
            return -1;
        }
        size_t l = strlen(str);
        if (l && str[l-1]=='\n')
            str[l-1]=0;

        /* split string into words*/
        int i;
        char* token = strtok(str, " \t\r\n");
        for (i=0; i<MAX_PARTS && token != NULL; i++)
        {
            arr[i] = token;
            token = strtok(NULL," \t\r\n");
        }

        fflush(stdin);

        /* check if the first word is quit*/
        if (strcmp(arr[0], quit)==0)
        {
            printf("Goodbye\n");
            break;
        }
        for (int j=0; j < i; j++){
            printf("pajaritos por aqui '%s'\n", arr[j]);
        }
    }

    return 0;
}

StackExchange Code Review Q#41460, answer score: 4