Help with String compare

[DarrelMan]

Making a command line interpreter for my Operating systems class

The following line is giving me a problem

if (strcmp(argv[0], "ren") == 0) {
        return RENAME;
        }

For whatever reason it does not return anything. However if I change "ren" to "darrel" it returns the proper value of RENAME and the command executes just fine and dandy. I get the same problem with "rename"

Is there something special with "ren"? I have even done an #undef ren just to make sure. Still does not play nice.

Any help at at all is greatly appreciated. If more code is needed I can make it available upon request.

[sanyam.mishra]

Give the declaration of argv

[DarrelMan]

It was declared in the header file as

char* argv[FILENAME_MAX];

[sanyam.mishra]

So basically its a one dimensional array. U cannot compare string stored in it like this
argv[0]== "ren"
Use
argv== "ren"

[DarrelMan]

It works for every other value though

for example:

if (strcmp(argv[0], "del") == 0) {
    return DELETE;
    }

Works just fine.

[sanyam.mishra]

I don't know how is it working for other values but it is against the basics. Are you sure it is the exact code? And suggested change should make the program work right.

[NathanOliver]

@ sanyam.mishra char* argv[FILENAME_MAX] is an array of pointers so it is a 2d array.

@ DarrelMan have you walked through the code with the debugger to see what is inside argv[0]? If you dont know how to step through the code with the debuger you could just output the contents of argv[0] to the console using a cout statement. That should at leat tell you what you have. it could be there is a slight mismatch.

[Schol-R-LEA]

DarrelMan: Did you mean to post this in the C forum? While it is certainly possible to write a shell in C++, most OS courses use vanilla C for such projects. If you are using C++, then you ought to be able to use the overloaded equality operator to compare the strings instead of strcmp(), which, given that you are looking for strict matches, should work for your purposes.

Mind you, if the goal is to write a shell, then I'm a bit confused over your variable naming. Conventionally, *argv[] is the name of a command line argument to the main() function, but beyond being a convention there's nothing special about the variable name argv. Still, one would normally assume that argv is a command argument to the current program, in which case argv[0] would be the name of the shell program itself, not the name of a program or shell operation. If, OTOH, you are using argv for the name of the array of strings which the shell reads in from stdin, and then parses to get the command line, then it is somewhat misleading to call it argv, as it can be confused with the shell's own argv argument.

To clarify what I mean: if your shell is named, say dmsh (for "DarrelMan's Shell"), then to have it running as a shell, you would run the program from the existing shell as

$ dmsh

Let's further assume you used ']' as the prompt (old-time Apple II users will get why I suggest this particular symbol). So, your shell would print:

]

And read in a command line. Let's say it's

] ren foo bar

In this case, you would have to somehow read the string as a whole as an array of sub-strings, each containing one element of the command. In that case, naming it argv, while understandable, is somewhat misleading. Conversely, if your program is just reading in a command from it's own command prompt:

$ dmsh ren foo bar

then argv[0] is going to be 'dmsh', not 'ren'.

[DarrelMan]

Schol-R-LEA

Good point probably should have posted to the C forum.

The assignment was given two static variables. argv[FILENAME_MAX] an array to store the string that was parsed and argc, an simple integer to used to store the amount of commands and parameters entered. The shell was largely written by the instructor, we just had to make 10 specified commands work.

So I initialze the shell and it displays my current working directory. It then presents a prompt for commands, and then it parses the string and returns the valid command to be run and runs the command.

    printf("%s>", cwd);//instructor provided line

    static char commandLine[_MAX_PATH];//instructor provided line

    argc = 0;  // instructor provided line
    argv[0] = NULL; // instructor provided line

            // TODO: Assignment 1 - Part 2 - Read command line, parse, store in argv---My code starts
    gets(commandLine);
    char * pch;
    pch = strtok (commandLine," /n");
    argv[0] = pch;
    argc++;
    while (pch != NULL) {
        pch = strtok (NULL, " /n");
        argv[argc] = pch;
        argc++;
        }
    if(argv[0] == NULL) {
        return NONE;
    } else {

        // TODO: Assignment 1 - Part 3 - Find and return command in commandTable that matches argv[0]

        if (strcmp(argv[0], "exit") == 0) {
            return EXIT;
        }
        if (strcmp(argv[0], "dir") == 0) {
            return DIR;
        }
        if (strcmp(argv[0], "copy") == 0) {
            return COPY;
        }
        if (strcmp(argv[0], "type") == 0) {
            return TYPE;
        } 
        if (strcmp(argv[0], "del") == 0) {
            return DEL;
        }
        if (strcmp(argv[0], "darrel") == 0) {   //this is the problem line.  "ren" "rename" "change"
            return RENAME;                      // do not work.  darrel does however, and so do the      
        }                                       // the other commands.    
        if (strcmp(argv[0], "mkdir") == 0) {
            return MKDIR;
        }
        if (strcmp(argv[0], "cd") == 0) {
            return CD;
        }
        if (strcmp(argv[0], "rmdir") == 0) {
            return RMDIR;
        }
    }
    return NONE;

I appreciate everyone's help so far. Thank you.

[Schol-R-LEA]

First off, I would strongly, emphatically recommend replacing the call to gets() with fgets(). The gets() function lacks boundary checking, and presents a substantial risk of buffer overflow.

Second, the delimiters for strtok() are incorrect; you presumably meant '\n' (the newline character) rather than '/' and 'n' (two separate characters). This would explain the problem you are experiencing, as right now it is truncating any word with an 'n' in it.

Third, in C, you can't have a statement followed by a declaration inside of a block. thus, you need to move the decalaration of pch to the line above the [f]gets().

Finally, I'll recommend a way of simplifying the program: in the declaration section, add the following declarations:

    int i;

    const struct {
        char* cmd;
        int code;
    } commands[] = {{"exit", EXIT}, {"dir", DIR}, {"copy", COPY}, {"type", TYPE}, 
                     {"del", DEL}, {"ren", RENAME}, {"rename", RENAME}, {"mkdir", MKDIR},
                     {"cd", CD}, {"rmdir", RMDIR}, {NULL, 0}};

You can then use this table as a lookup table for the commands, and loop through it like so:

    // TODO: Assignment 1 - Part 3 - Find and return command in commandTable that matches argv[0]
    for (i = 0; commands[i].cmd != NULL; i++) {            
        if (strcmp(argv[0], commands[i].cmd) == 0) {
            return command[i].code;
        }
    }

    return NONE;
}

(This assumes that the command codes are integers; if they are an enumeration of some kind, you would want to change the declaration of code to match.)

[DarrelMan]

Got it! Thank you! LOL it's amazing what a wrong facing slash does.

I also implemented your other suggestions. Thanks a million.