How can I avoid using a global variable?

Question

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;
}

We need more infos about context. Otherwise is too broad, but I'm afraid will always be too broad... — LPs, Jun 06 '16 at 08:31
Side note: tons of heap allocated vars (`strdup`, `malloc`, ..) without a single `free`... — LPs, Jun 06 '16 at 08:42
Instead of assigning only C to A, maybe you can assign some structure containing both B and C. — August Karlstrom, Jun 06 '16 at 09:40

score 1 · Answer 1 · answered Jun 06 '16 at 09:21

Im not expert but usually the answer is straight forward. if you dont want to use global variables (which is often a good idea) then you need to pass the local variable as a reference to the functions you want to use it in and update the calling function (usually in main()).

score 1 · Accepted Answer · answered Jun 06 '16 at 09:22

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;
}

With `static` **it is global** in the module where the variable is declared. AFAIK [tag:c] has no such a _member variables_... — LPs, Jun 06 '16 at 09:37
Nor does it have inheritance, overloading, etc. And yet you can still implement all your favorite design patterns. Don't let the language limit you. — Illishar, Jun 06 '16 at 09:46

How can I avoid using a global variable?

2 Answers2