Programming Forums
User Name Password Register
 

RSS Feed
FORUM INDEX | TODAY'S POSTS | UNANSWERED THREADS | ADVANCED SEARCH

Reply
 
Thread Tools Display Modes
Old Sep 7th, 2007, 1:05 AM   #1
dayrinni
Newbie
 
Join Date: Apr 2005
Posts: 17
Rep Power: 0 dayrinni is on a distinguished road
exec functions

Hello,

I'm trying to write a Linux shell for one of my classes. I'm having some logic problems with getting exec to work.

I am looking to use exec in the following manner:
1.) Only provide the name of the program rather than the fill path. So this means I use either: execlp or execvp.
2.) I read a string of input from the command prompt and then I do the following on it: parse it until the first space and that is placed into a command variable. I will continue this process and placing them into different variables, so I can send it to exec*.

I am interested in using execvp. I think this would be the easiest way of doing things for my shell. So, what I need to do is I need to parse the input and break it up into tokens. I know a way to do this, but it is really really really icky.

My code looks something like this...
Declare ptr char array.
Declare temp buffer: char temp[SIZE];
Declare counter for the temp.
Declare counter for the array (where we put temp after we come to a space).

Loop for the size of the command line:
if current character is NOT a space
    place into temp at temp's counter: ie: temp[temp_counter] = command_line[command_line_counter];
else
    place temp into the array
    increment array counter
    reset temp's counter to zero
    zero out temp with '\0'

This kind of works. I am having reference problems with the array holding the last reference of the temp buffer. I've used strcpy and that doesn't work too well. So I haven't been able to get past that. If anyone has any ideas that'd be cool.

Thanks,

PS: I would change the name of post if I could since I named it incorrectly.

Last edited by dayrinni; Sep 7th, 2007 at 1:19 AM.
dayrinni is offline   Reply With Quote
Old Sep 7th, 2007, 2:55 AM   #2
dr.p
Programmer
 
dr.p's Avatar
 
Join Date: Feb 2006
Location: Ohio
Posts: 93
Rep Power: 3 dr.p is on a distinguished road
If you had posted actual code, instead of pseudo-code, I might be able to help. As it is, I can only suggest that you make sure to terminate your temp string with "\0" before you use strcpy on it. That's the only thing I don't see mentioned in your pseudocode.
__________________
Neeley.org
dr.p is offline   Reply With Quote
Old Sep 7th, 2007, 3:15 AM   #3
dayrinni
Newbie
 
Join Date: Apr 2005
Posts: 17
Rep Power: 0 dayrinni is on a distinguished road
Quote:
Originally Posted by dr.p View Post
If you had posted actual code, instead of pseudo-code, I might be able to help. As it is, I can only suggest that you make sure to terminate your temp string with "\0" before you use strcpy on it. That's the only thing I don't see mentioned in your pseudocode.
I didn't want to post code so I would not give the impression I am trying to not do any work.

I'll write up a test program and then you can help me with it. I'm having a large problem with copying the char's over to the ptr array...stay tuned for code.

Thanks
dayrinni is offline   Reply With Quote
Old Sep 7th, 2007, 3:30 AM   #4
dayrinni
Newbie
 
Join Date: Apr 2005
Posts: 17
Rep Power: 0 dayrinni is on a distinguished road
Alright, here is what I have right now. This is really horrible code, I've been trying things out and the flow of the program isn't very good atm. If I can get the contents into the arglist without being corrupted and in the correct order...then hopefully I can move on to writing the rest of this thing.

I wrote a ton of C code about 5-6 years ago, but been using Java lately and it has spoiled me horribly. Anyways, I'm at the point where I'm making no headway but I don't want to stop...but I should so I guess I will until tomorrow.

I ran this on my schools Linux box. Type "ls -al" once and then type it again. You should see the corruption of the memory.

Thanks for any help.



#include "stdio.h"
#include "stdlib.h"

#include <string.h>
#include <unistd.h>
#include <sys/types.h>


int mainLoop();
void getCommandLineInput();
void loadCommands();
void executeCommand(char* command, char* commandLine);
void executeCommand2(char* command, char* commandLine);

const int MAX_BUF = 30;
const int MAX_ARGUMENTS = 5;

char commandLineInput[MAX_BUF];


int main()
{
        return mainLoop();
}

int mainLoop()
{
        int exitFlag = 0;
        bool run = true;

        while(run)
        {
		printf("Type in a string to be parsed. Type exit to quit.\n");
                getCommandLineInput();


                //We need to break the input up to isolate the exact command.
                //Iterate until a space is located.
                char command[MAX_BUF];
                char arguments[MAX_BUF];
                bool writeCommand = true;
                int arg_counter = 0;

                for(int i = 0;i<MAX_BUF;i++)
                {
                        arguments[i] = '\0';
                        command[i] = '\0';

                        if(writeCommand == true && commandLineInput[i] != ' ')
                                command[i] = commandLineInput[i];
                        else
                        {
                                if(writeCommand)
                                        writeCommand = false;
                                else
                                {
                                        arguments[arg_counter] = commandLineInput[i];
                                        arg_counter++;
                                }
                        }
                }

                printf("The command is: %s\n", command);
                printf("The arguments are: %s\n", arguments);

                if(!strcmp(commandLineInput, "exit"))
                {
                        printf("Exiting\n");
                        break;
                }


                executeCommand(command, arguments);
        }


        return exitFlag;
}

//Gets the input from stdin and puts it into the global variable.
void getCommandLineInput()
{

        gets(commandLineInput);

        //printf("%s\n", commandLineInput);

}

void executeCommand(char *command, char* commandLineInput)
{
        char *arglist[MAX_ARGUMENTS];

        int size = strlen(commandLineInput);
        char temp[size];
        int i;
        for(i = 0;i<size;i++)
                temp[i] = '\0';
        for(i = 0;i<MAX_ARGUMENTS;i++)
                arglist[i] = new char;

        arglist[0] = command;
        int temp_counter = 0;
        int arg_counter = 1;

        bool quote = false;
        for(i = 0;i<size;i++)
        {
		//Put the characters in the temp buffer if its not a space or if we are in a quote
                if(commandLineInput[i] != ' ' || quote == true)
                {
                        printf("looking at: %c\n", commandLineInput[i]);
                        temp[temp_counter] = commandLineInput[i];
                        temp_counter++;

			//ok reached either the beginning or the end so negate it.
                        if(commandLineInput[i] == '\"')
                                quote = !quote;
                }
                else
                {
                        int j;

			//horrible code - i really can't figure this out.
			//i've tried strdup, sprintf, strcpy, strncpy, and manually copying the contents over but it still gives me problems.

                        temp[temp_counter] = '\0';
                        char *temp2 = strdup(temp);

			//display the results and add in terminators.
                        for(j=temp_counter;j<size;j++)
                        {
                                temp[temp_counter] = '\0';
                                printf("temp[%d]: %c\n", j, temp[j]);
                        }


			//set the temp2 to the arglist. temp2 is not referenced to the other items in the arglist array.
                        arglist[arg_counter] = temp2;

			//display some debug information
                        printf("temp2: %s\n", temp2);
                        printf("temp: %s\n", temp);
                        printf("arglist: %s\n", arglist[arg_counter]);

			//result the counters
                        arg_counter++;
                        temp_counter = 0;

                        //zero out temp
                        for(j = 0;j<size;j++)
                                temp[j] = '\0';
                }
        }


	//since the loop terminates before it puts in the last item, we manually do it here.
        printf("temp: %s\n", temp);
        arglist[arg_counter] = temp;
        arg_counter++;

	//display the contents
        for(i =0;i<arg_counter;i++)
        {
                printf("arg: %s\n", arglist[i]);
        }


	//copy it to a new arg list because exec will exec the commands the number of times according to the size of arglist.
	//so with the way this currently works, we dont know the size of the commands before we allocate the arglist var above.


        //new arg list.
        char * arglist2[arg_counter+1];

	//reset the memory locations
        for(i = 0;i<(arg_counter+1);i++)
                arglist2[i] = new char;

	//display some debug info
        printf("arg counter +1: %d\n", (arg_counter+1));

	//copy the contents over.
        for(i = 0;i<(arg_counter+1);i++)
        {
                arglist2[i]=arglist[i];
                printf("arg2: %s\n", arglist2[i]);
        }

        printf("Command: %s\n", command);


	return;

	//execute the command
	pid_t pid;

        pid = fork();

        if (pid < 0)
        {
                fprintf(stderr, "Fork failed.\n");
                exit(2);

        }
        if (pid == 0)
        {
              if( execvp(command, arglist) == -1)
                {
                        fprintf(stderr, "Execl Failed\n");
                        exit(3);

                }
        }
        printf("Parent(shell) continues.\n");
	
}
dayrinni is offline   Reply With Quote
Old Sep 7th, 2007, 4:09 AM   #5
dr.p
Programmer
 
dr.p's Avatar
 
Join Date: Feb 2006
Location: Ohio
Posts: 93
Rep Power: 3 dr.p is on a distinguished road
Yeah, that code's not real pretty... but that's what happens when you're learning

Those places where you're assigning a "new char" to the arglist array entries is completely pointless. You're allocating a new character in memory, assigning the pointer to an array entry, and then overwriting that pointer later, which is a memory leak.

That, actually, may be part of your problem, as well. The execvp docs say that the array of pointers must end with a NULL pointer.

So, you would want to end up with:
arglist[0] = pointer to arg1 string
arglist[1] = pointer to arg2 string
arglist[2] = pointer to arg3 string
arglist[3] = NULL (literally set it to NULL)
arglist[4+] = invalid and ignored

I also don't understand your use of arglist2 at all. It looks unnecessary.

You also have another memory leak. You're allocating memory by calling strdup (which is completely okay in this situation,) but you're never de-allocating the pointers stored in arglist[] by calling free().

EDIT: I went and compiled your code. The "char temp[size];" line in your code is the cause of your problem. You either need to use a constant value to initialize the temp array (e.g. char temp[256]) or allocate the memory for it (e.g. char *temp = new char[size]).
__________________
Neeley.org

Last edited by dr.p; Sep 7th, 2007 at 4:49 AM.
dr.p is offline   Reply With Quote
Old Sep 7th, 2007, 5:15 AM   #6
lectricpharaoh
SEXY SHOELESS GOD OF WAR!
 
lectricpharaoh's Avatar
 
Join Date: Jun 2005
Location: Wet west coast of Canada
Posts: 1,198
Rep Power: 5 lectricpharaoh will become famous soon enough
Quote:
Originally Posted by dayrinni
//Gets the input from stdin and puts it into the global variable.
void getCommandLineInput()
{
        gets(commandLineInput);
        //printf("%s\n", commandLineInput);
}
I'm not sure what your problem is, as I've only glanced over this thread, but you really should not use gets(). It is evil. The problem is that there is no mechanism for specifying the size of the buffer, thus if the user enters too many characters, it will overflow.

The solution is to use fgets() instead, and specify stdin as the stream:
// gets user input from stdin and puts it into global buffer.  returns 1 on success, and 0 on failure.
int getCommandLineInput()
{
  printf("> ");  // CLI input prompt
  fflush(stdout);  // flush the stream to force output of the prompt
  if(fgets(commandLineInput, MAX_BUF, stdin) != NULL)
    return 1;  // success
  else
    return 0;  // failure
}
I've also changed the function so it now returns an int, so you can check for success/failure of the input operation. You might consider adopting a similar approach. Knowing when things go awry in your programs is a good thing; ignorance in this case is most certainly not bliss.
Quote:
Originally Posted by dr.p
EDIT: I went and compiled your code. The "char temp[size];" line in your code is the cause of your problem. You either need to use a constant value to initialize the temp array (e.g. char temp[256]) or allocate the memory for it (e.g. char *temp = new char[size]).
It's C, not C++, so new isn't available (gotta use malloc() or its siblings). Still, initializing an array's size with a const int may cause problems. I believe this is an allowable practice in the C99 standard, but I'm not 100% certain. If the fellow's using an older compiler, however, I'm pretty sure the standard doesn't allow it.

@OP: If this turns out to be the source of your problem, simply use a #define directive rather than a const int definition.

[edit] My bad. I was thinking of MAX_BUF. The variable size is a simple int, not even const, so it will not work in any case. Use #define instead, or const int if your compiler allows that. [/edit]
__________________
And once again, Probability proves itself willing to sneak into a back alley and service Drama as would a copper-piece harlot.
- Vaarsuvius, Order of the Stick

Last edited by lectricpharaoh; Sep 7th, 2007 at 5:29 AM.
lectricpharaoh is offline   Reply With Quote
Old Sep 7th, 2007, 5:42 PM   #7
dayrinni
Newbie
 
Join Date: Apr 2005
Posts: 17
Rep Power: 0 dayrinni is on a distinguished road
Hi guys,

Thanks a lot for your help, your suggestions have solve the problem with putting the contents into the arglist and then exec is now functioning correctly.

I made a slew of silly mistakes, with the defines, haha. Too much Java
dayrinni is offline   Reply With Quote
Old Sep 7th, 2007, 8:08 PM   #8
dayrinni
Newbie
 
Join Date: Apr 2005
Posts: 17
Rep Power: 0 dayrinni is on a distinguished road
Hi,

I'm looking to turn that function into a more generic one, where it will return a ptr to an array of chars. I'm not entirely sure on what the return type should be because char*[] func(); doesn't work. Or will char * be sufficient enough and I can treat it as a char*[]?

I suppose if this is not possible (tried a few things), I will make the arglist global, like I did with the commandinput.

Thanks!
dayrinni is offline   Reply With Quote
Old Sep 7th, 2007, 8:39 PM   #9
lectricpharaoh
SEXY SHOELESS GOD OF WAR!
 
lectricpharaoh's Avatar
 
Join Date: Jun 2005
Location: Wet west coast of Canada
Posts: 1,198
Rep Power: 5 lectricpharaoh will become famous soon enough
Quote:
Originally Posted by dayrinni View Post
Hi,

I'm looking to turn that function into a more generic one, where it will return a ptr to an array of chars. I'm not entirely sure on what the return type should be because char*[] func(); doesn't work. Or will char * be sufficient enough and I can treat it as a char*[]?

I suppose if this is not possible (tried a few things), I will make the arglist global, like I did with the commandinput.

Thanks!
It's entirely possible. Use char ** as your return type:
#include <stdio.h>

char **arrayOfCharPointers(void);

int main(void)
{
  char **array = arrayOfCharPointers();
  while(*array != NULL)
    puts(*(array++));
  system("pause");
  return 0;
}

char **arrayOfCharPointers(void)
{
  static char *array[5] = {"one", "two", "three", "four", NULL};
  return array;
}
Note I've made the local array static; this prevents the memory from being released when the arrayOfCharPointers() function returns. You'll probably instead want to use malloc() or some such for a function like this.
__________________
And once again, Probability proves itself willing to sneak into a back alley and service Drama as would a copper-piece harlot.
- Vaarsuvius, Order of the Stick
lectricpharaoh is offline   Reply With Quote
Old Sep 7th, 2007, 11:34 PM   #10
dayrinni
Newbie
 
Join Date: Apr 2005
Posts: 17
Rep Power: 0 dayrinni is on a distinguished road
Quote:
Originally Posted by lectricpharaoh View Post
It's entirely possible. Use char ** as your return type:
#include <stdio.h>

char **arrayOfCharPointers(void);

int main(void)
{
  char **array = arrayOfCharPointers();
  while(*array != NULL)
    puts(*(array++));
  system("pause");
  return 0;
}

char **arrayOfCharPointers(void)
{
  static char *array[5] = {"one", "two", "three", "four", NULL};
  return array;
}
Note I've made the local array static; this prevents the memory from being released when the arrayOfCharPointers() function returns. You'll probably instead want to use malloc() or some such for a function like this.
Thanks for your reply.

For the malloc part - I'd allocate memory based on the size of what I'd like to put in the index?
ie: If my command input was:
cp testfile.txt mynewtestfile.txt

and my arglist would be this after the parse:
0: cp
1: testfile.txt
2: mynewtestfile.txt
3: NULL

I would allocate memory for each index (except NULL) for their different sizes?

arglist[i] = malloc(n)?

Thanks.
dayrinni is offline   Reply With Quote
Reply

Bookmarks

« Previous Thread in Forum | Next Thread in Forum »

Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
 
Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Forum Jump

Similar Threads
Thread Thread Starter Forum Replies Last Post
C# corruption!!! Kilo C++ 32 May 21st, 2006 9:44 PM
binding between ordinary member functions & polymorphic member functions ASH C++ 2 May 11th, 2006 5:36 AM
Buttons!! Sane Python 7 Jul 6th, 2005 10:04 PM
Exporting Functions victorsk Visual Basic 1 May 18th, 2005 3:32 PM
User-defined creatNode and deleteNode functions for a doubly-linked list jgs C 2 Apr 28th, 2005 9:53 AM




DaniWeb IT Discussion Community
All times are GMT -5. The time now is 1:12 AM.

Powered by vBulletin® Version 3.7.0, Copyright ©2000 - 2008, Jelsoft Enterprises Ltd.
Copyright ©2007 DaniWeb® LLC