cglobal-variableslemon

How can I avoid using a global variable?


Is it ever ok to use a global variable? I am using a global variable and I understand that is not ideal. It looks like this

int result2 = 0;

void setresult2(int a) {
    result2 = a;
}

I'm using the variable in my lemon grammar for my custom shell:

expr(A) ::= IF LSBR expr(B) RSBR SEMICOLON THEN expr(C) SEMICOLON FI. { setresult2(B); A=C; }

Now I wonder if there is a better way than using a global variable?

The entire code is available on my github. The purpose of the global variable is to handle an if statement in shell expansions and shell script so that my shell can read and execute an if statement.

The code in context is the parsing of the if statement that uses the grammar that updated the global variable, for the only way to communicate between the grammar and my C:

char *if_execute(char *shellcommand) {

    char mystring[CMD_LEN];
    void *pParser;
    char *c;
    int reti;
    shellcommand = str_replace(shellcommand, ";", " ; ");
    reti = regcomp(&regex, "[0-9]==[0-9]", 0);
    if (reti) {
        fprintf(stderr, "Could not compile regex\n");
        exit(1);
    }

    /* Execute regular expression */
    reti = regexec(&regex, shellcommand, 0, NULL, 0);
    if (!reti) {
        shellcommand = str_replace(shellcommand, "==", " == ");;
    }
    else if (reti == REG_NOMATCH) {
        /* puts("No match"); */
    }
    else {
        regerror(reti, &regex, msgbuf, sizeof(msgbuf));
        fprintf(stderr, "Regex match failed: %s\n", msgbuf);
        exit(1);
    }

    char *line = strcpy(mystring, shellcommand);
    pParser = (void *) ParseAlloc(malloc);
    if (line) {
        char *buf[64];
        struct SToken v[32];
        int value;
        char **ptr1 = str_split(buf, line, ' ');
        int j = 0;
        for (j = 0; ptr1[j]; j++) {
            c = ptr1[j];
            char *c2 = strdup(c);
            if (c2 == NULL) {
                perror("strdup");
                exit(EXIT_FAILURE);
            }
            v[j].token = c2;
            switch (*c2) {
                case '0':
                case '1':
                case '2':
                case '3':
                case '4':
                case '5':
                case '6':
                case '7':
                case '8':
                case '9':
                    for (value = 0; *c2 && *c2 >= '0' && *c2 <= '9'; c2++)
                        value = value * 10 + (*c2 - '0');
                    v[j].value = value;
                    Parse(pParser, INTEGER, &v[j]);
                    continue;
            }

            if (!strcmp("if", c)) {
                Parse(pParser, IF, NULL);
            }
            else if (!strcmp("true", c)) {
                Parse(pParser, TRUE, NULL);
            }
            else if (!strcmp("then", c)) {
                Parse(pParser, THEN, NULL);
                char *token = "then ";
                const char *p1 = strstr(shellcommand, token) + strlen(token);
                const char *p2 = strstr(p1, ";");
                if (p2 == NULL) {
                    // TODO: Handle it
                }
                size_t len = p2 - p1;
                char *res = (char *) malloc(sizeof(char) * (len + 1));
                if (res == NULL) {
                    fprintf(stderr, "malloc failed!\n");
                }
                strncpy(res, p1, len);
                res[len] = '\0';

                if (result2)
                    shellcommand = res;
                else
                    shellcommand = "echo";
            }
            else if (!strcmp("[", c)) {
                Parse(pParser, LSBR, NULL);
            }
            else if (!strcmp("]", c)) {
                Parse(pParser, RSBR, NULL);
            }
            else if (!strcmp(";", c)) {
                Parse(pParser, SEMICOLON, NULL);
            }
            else if (!strcmp("fi", c)) {
                Parse(pParser, FI, NULL);
            }
            else if (strlen(c) > 0 && strstr(c, "==")) {
                v[j].token = c;
                Parse(pParser, EQEQ, &v[j]);
            }
            else {
                Parse(pParser, FILENAME, NULL);
            }
        }
        Parse(pParser, 0, NULL);
    }

    return shellcommand;
}

Solution

  • I'd say that "member variables" can be ok in the un-generic parts of your code. Eg.

    static int member_var1 = 0;
    
    int get_var1(void)
    {
       return member_var1;
    }
    

    The "static" keyword will make sure that your member variable stays a member variable and not becomes a global variable.

    The next issue is then how to identify what is generic code and what is not. I'm not sure that there's any bulletproof methods for this. Your main file rarely makes sense to reuse or overload. (So go ahead and put member variables into that.)

    For anything else, I'd prefer something like this:

    struct my_state
    {
       int var1;
    }
    
    int do_some_function(struct my_state* state, int some_value)
    {
       state->var1 = 0;
    }