C question for homework (easy)

[Danimal1209]

Hi guys, I'm having trouble with a couple problems in C. I am newish to C as I have taken several classes on Java, but C is a lot different in passing by value.

Anyway here is what I have. The homework assignment is a pre-written shell that I need to modify to do things. Anyway...

void eval(char *cmdline) //This is fine
{
char array[MAXLINE];
array[0] = *cmdline;
if(parseline(*cmdline, *array)== 1) //This is the problem
}

int parseline(const char *cmdline, char **argv) //This is fine

I have a terrible time with pointers in C, I get extremely confused by them.
The first argument parseline takes is a pointer, which I am passing it. The second argument it takes is a pointer to a pointer, and I don't know what to pass it. Do I need to make another local variable in eval that points to array and then point to that?

Like
char *newArray = &array; ????

 

[iCyborg]

When you declare a variable, * denotes a pointer. When you use it later, it means dereferencing, i.e. returning the value that is pointed to, the opposite of & operator that takes the address.
Your 'array' is of type 'char *', when you do *array, you will get char. To get char** you need to take the address of 'array' (which, as already said, is a pointer). You're also passing cmdline wrong: parseline() expects a char*, cmdline is already a char*, no need to do anything to it, especially dereference it.
You should perhaps revisit the concept of pointers and addresses, there should be some good online sources, and it will be clearer to you.

 

[postmortemIA]

yeah, you need to re-learn pointers, and character arrays
you can't get far in C without mastering the pointers..

1. c-strings are copied using strcpy function
2. array[0] denotes 1st item in array (a single character), array itself can be treated as a pointer.

 

[Danimal1209]

that helps, but now im getting a segmentation fault. I think the index im referencing is out of bounds. I've altered the functions to this:

void eval(char *cmdline)
{
  char *array[MAXLINE];
  if(parseline(cmdline, array)== 1) //1 = no arguments
  {
    if(builtin_cmd(array)== 0) //if array is not builtin, return to main
       {
         return;
       }
    return;                    //return to main after builtin is run
  }
  return;                      //return to main
}



int builtin_cmd(char **argv)
{
 //FAULT OCCURS HERE// if(strcmp(argv[0], "quit")== 0)  //if command was quit, exit program
    {
      exit(0);
    }
    return 0;     /* not a builtin command */
}

GDB is telling me the fault is occurring at the line specified in builtin_cmd. I am assuming I don't have the right type of argument for strcmp. Would that be accurate?

 

[iCyborg]

I don't see anything wrong with the given code, so the error is probably in parseline function that is supposed to initialize array[0], but probably doesn't. Wrong type would cause a compiler error, I think the most likely cause is that argv[0] (i.e. array[0]) is NULL and you get segfault when strcmp tries to dereference it. array[0] might also be garbage which would also cause segfault/access violation.

 

[EagleKeeper]

char *array[MAXLINE]

defines an array of pointers, but they are pointing to garbage. Not set to anything. Until you assign a value to them, using them as a pointer is invalid.

You also do not verify against null as Cyborg mentioned above.

Two critical errors on code exist

• Not initializing variables.
• Not testing bounds before use as pointer or variable

 

[iCyborg]

I think the idea behind parseline function is to take one command line (in char* cmdline) and then parse it into tokens in char** array. He probably tries to initialize there, but that code is not provided, so we can only guess what happens there.

 

[Danimal1209]

Here is the parseline function.

It was written by the prof, so we aren't supposed to alter it.
It is somewhat hard for me to follow, but it looks like later in the function it initializes argv[0] to null, which would cause a problem.

What do you guys think?

int parseline(const char *cmdline, char **argv)
{
    static char array[MAXLINE]; /* holds local copy of command line */
    char *buf = array;          /* ptr that traverses command line */
    char *delim;                /* points to first space delimiter */
    int argc;                   /* number of args */
    int bg;                     /* background job? */

    strcpy(buf, cmdline);
    buf[strlen(buf)-1] = ' ';  /* replace trailing '\n' with space */
    while (*buf && (*buf == ' ')) /* ignore leading spaces */
        buf++;

    /* Build the argv list */
    argc = 0;
    if (*buf == '\'') {
        buf++;
        delim = strchr(buf, '\'');
    }
    else {
        delim = strchr(buf, ' ');
    }

    while (delim) {
        argv[argc++] = buf;
        *delim = '\0';
        buf = delim + 1;
        while (*buf && (*buf == ' ')) /* ignore spaces */
               buf++;

        if (*buf == '\'') {
            buf++;
            delim = strchr(buf, '\'');
        }
        else {
               delim = strchr(buf, ' ');
        }
    }
        argv[argc] = NULL;

    if (argc == 0)  /* ignore blank line */
        return 1;

    /* should the job run in the background? */
    if ((bg = (*argv[argc-1] == '&')) != 0) {
        argv[--argc] = NULL;
    }
    return bg;
}

 

[iCyborg]

So 'array' is indeed initialized inside parseline, and each item in the array is a pointer to some position in the starting cmdline, so there's no need to allocate anything for 'array' variable. So far, so good.

One thing that strikes me as wrong is that you have a comment in the line where you compare the result of parseline to 1 and the comment is "//1 = no arguments".
Looking at the code, this is wrong: for a blank line you get 1, in this case no commands nor arguments. But in any regular case, parseline returns 'bg' which says whether the process is background or not, and not the number of parsed tokens or arguments. There is a way to get this number of tokens, but being a homework, I will let you think about that for a while.

I c/p-ed the code into VS, and setting cmdline="quit &\n" (*) and calling eval(cmdline), I do not get segfault.
I think the only way this can happen is if you set cmdline = "" i.e. an empty string, but a string nonetheless. This is wrong since argv[0] will then be NULL and you're not testing for this, and as said, I think you have a misunderstanding of what parseline's return result means.

(*) parseline expects '\n' always and strips it, so must have this, and I need to add '&' as well or your code will not call builtin_cmd().

 

[Danimal1209]

I got that part figured out. I had == 0 when I needed != 0.

NEXT PART!!

I need to check if a variable start with a % and then an int. I was looking around for a function that does this, but I can only find ones that look to see if the first char is a specific char.

So I need to find a function to error check if an argument entered with fg start with a % and then an int.

 

[iCyborg]

There are no standard functions that will do what you want, so you'll have to write it yourself. There are functions that can be useful here - isdigit() tests that something is a digit, not that it's much effort to write your own.

 

[Cogman]

There are no standard functions that will do what you want, so you'll have to write it yourself. There are functions that can be useful here - isdigit() tests that something is a digit, not that it's much effort to write your own.

Now might be a good time to introduce http://www.pcre.org/

A regular expression will do what you want and probably better than you would do it.

 

[Markbnj]

Or you could call the function that tests for a specific char twice and adjust the char* to point to the second char for the second call.

 

[iCyborg]

Now might be a good time to introduce http://www.pcre.org/

A regular expression will do what you want and probably better than you would do it.

Kind of shooting flies with machine guns
Even for a commercial product, it would need to be more than just a bit of parsing before I would make a case for getting all that code, learning the interface, setting up the project to compile it, link it etc. And this is only a homework.
Thanks though for the link, it''s good to know about it.

 

[iCyborg]

Or you could call the function that tests for a specific char twice and adjust the char* to point to the second char for the second call.

He needs to parse %n where n is an integer, and could be more than 1 digit - seems like he's parsing Linux commands, and %n brings job/process number n to the foreground, so n can be any integer.

 

[veri745]

Strip the first char and use atoi() ?

 

[Cogman]

Kind of shooting flies with machine guns
Even for a commercial product, it would need to be more than just a bit of parsing before I would make a case for getting all that code, learning the interface, setting up the project to compile it, link it etc. And this is only a homework.
Thanks though for the link, it''s good to know about it.

When I destroy things, I destroy them well .

Seriously though, the amount of code written to get PCRE up and running will be slightly more than the code needed to match this specific pattern. PCRE is common enough as are RegExs that it could be useful to learn both.

 

[iCyborg]

Strip the first char and use atoi() ?

It has crossed my mind, and he can use it to convert string to int, but I think he will still need to parse it manually. atoi() is pretty lax about the format, as long as there's some number inside it's ok (say atoi("23xd") = 23), and it will allow -3 etc.

 

[Danimal1209]

I'm not very good at programming, I just need to pass this class. I hate it.

Do I need to do something like this?

take the first char and make it a string
take the rest of the chars and put them in a string
test first string to see if it is a "%"
convert second string to int
if second string cannot convert then call error message

Would this work fine?
Is it possible?

 

[Markbnj]

He needs to parse %n where n is an integer, and could be more than 1 digit - seems like he's parsing Linux commands, and %n brings job/process number n to the foreground, so n can be any integer.

Ah yeah, not that simple then.

 

[EagleKeeper]

As someone said above; using atoi passing in the pointer to the first charactger after the % will generate a decimal number; atoi terminates on the first non-digit characdter and returns what every it parsed out.

%50x will return the decimal value of 50

 

[Danimal1209]

argv[0] would be me typing in "fg" (foreground) in to the prompt

argv[1] would be the argument with fg, for example, if I wanted to bring a job to the foreground with a job ID number of 23

prompt> fg %23

so, argv[0] = "fg"

argv[1] = "%23"

and argv[2] = 0

So, how can I point to the character after the % ?

 

[iCyborg]

Remember that C string is just a char pointer. It's a variable whose content is a memory address of the first character of the string and the string continues until you get to 0 (not character 0 which is ascii/utf coded, but real 0). So argv[1] = "%23" really is just the mem address that has % in it. You want the memory address where 2 is - the string starting from there will end at the same spot as the original string since they all go until \0 is found.

Nice thing about pointers in C/C++ is that ptr+1 will point to the memory address of the next member of the underlying type, so if you have plain C char like here, it will add 1 byte and point to the next character, if it's a 12 byte struct, it will add 12 bytes.

If you look at your prof's code and try to understand it, it would give you some clue for this too.

 

[EagleKeeper]

argv[0] would be me typing in "fg" (foreground) in to the prompt

argv[1] would be the argument with fg, for example, if I wanted to bring a job to the foreground with a job ID number of 23

prompt> fg %23

so, argv[0] = "fg"

argv[1] = "%23"

and argv[2] = 0

So, how can I point to the character after the % ?

char *ptr = argv[1]; // %
ptr++; // *ptr is 2

itoc(ptr) will produce an output of 23