Redirection issue

[souissipro]

Hi,

I have written a C program that does some of the functionalities
mentionned in my previous topic posted some days ago. This shell
should:
1- execute input commands from standard input, and also from a file
conatining the commands
2- does the redirection of the input and output from and to files.
3- retrieve the environment variables like HOME,..

I did the first question and it works fine, however when I modified the
code to implement the redirection it does not work anymore.

Could you please have a look to my code and tell me which is wrong
with?

I would also appreciate if you can tell me how to implement the 2nd
question in the function parse_line (see the code below):

Thank you very much for your help on this

Souissipro:

---------------SHELL---------------------------
#include<stdio. h>
#include<stdlib .h>
#include<string .h>
#include<unistd .h>
#include<fcntl. h>

/* maximal number of command arguments */

#define NARGS 50
#define MAXL 255

static char *prompt = "$ ";
static char *sep = " \n";

int
do_exit(char *argv[])
{
if (argv[1]==NULL)
exit(EXIT_SUCCE SS);
else
fprintf(stderr, "Error: don't take arguments.\n");
return 1;
}

int
simple_cmd(char *argv[])
{
int i;
int pid, status;

if (strcmp(argv[0],"exit")==0)
return do_exit(argv);
for(i=0;argv[i]!=NULL;i++)
printf("argv[%i]=\"%s\";\n",i,a rgv[i]);

if (strcmp(argv[0],"cd")==0)
{
return chdir(*argv);
}

/*
* get the child process.
*/
if ((pid = fork()) < 0) {
perror("fork");
exit(1);
}

/*
* child execute code inside if.
*/
if (pid == 0) {
execvp(*argv, argv);
perror(*argv);
exit(1);

}

/*
* parent execute wait.
*/
while (wait(&status) != pid)
/* empty */ ;
}

int
redir_cmd(char *argv[],char *in, char *out)
{
int i;
int pid, status;
int fd1, fd2;
int dummy;

if (strcmp(argv[0],"exit")==0)
return do_exit(argv);
for(i=0;argv[i]!=NULL;i++)
printf("argv[%i]=\"%s\";\n",i,a rgv[i]);

if (strcmp(argv[0],"cd")==0)
{
return chdir(*argv);
}

if (fork() == 0) {
fd1 = open(in, O_RDONLY);
if (fd1 < 0) {
perror("can't open file : in");
exit(1);
}

if (dup2(fd1, out) != 0) {
perror("catf1f2 : dup2(in, 0)");
exit(1);
}
close(fd1);

fd2 = open(out, O_WRONLY | O_TRUNC | O_CREAT, 0644);
if (fd2 < 0) {
perror("catinou t: out");
exit(2);
}
/*
if (dup2(fd2, 1) != 1) {
perror("catf1f2 : dup2(out, 1)");
exit(1);
}
*/
close(fd2);

execvp(*argv, argv);
perror(*argv);
exit(1);

}

/*
* parent executes wait.
*/
else {
wait(&dummy);
}

}

int
parse_line(char *s,char *in, char *out)
{
char *argv[NARGS], *cur, *old;
int i=0;

if ((s[0]=='#')||(s[0]=='\n'))
return 0;
old=cur=s;
while((cur=strp brk(cur,sep))!= NULL) {
argv[i++]=old;
cur[0]='\0';
cur++;
old=cur;
}
argv[i]=NULL;
if
return redir_cmd(argv, in, out);
}

/* Main function
*/

int
main(int argc, char *argv[])
{
char s[MAXL];

int i;
char buf[255];
FILE *f;

if (argc 3) {
fprintf(stderr, "incorrect arguments \n");
exit(EXIT_FAILU RE);
}
if (argc == 2) {
f = fopen(argv[1], "r");
if (f == NULL) {
fprintf(stderr, "impossible to open the file for reading\n");
exit(EXIT_FAILU RE);
}

while (fgets(buf, 255, f) != NULL) {
parse_line(buf, argv[1],"stdout");
printf("%s",buf );
}
fclose(f);
}
if (argc == 3) {
f = fopen(argv[1], "r");
if (f == NULL) {
fprintf(stderr, "impossible to open the file for reading\n");
exit(EXIT_FAILU RE);
}

while (fgets(buf, 255, f) != NULL) {
parse_line(buf, argv[1],argv[2]);
printf("%s",buf );
}
fclose(f);
}
if (argc == 1) {

printf("vsh - mini-shell5.\n\n%s", prompt);
while(fgets(s,M AXL,stdin)!=NUL L) {
parse_line(s,"s tdin","stdout") ;
printf("%s",pro mpt);
}
}
return EXIT_FAILURE;
}

[Spiros Bousbouras]

Re: Redirection issue

souissipro wrote:

Hi,
>
I have written a C program that does some of the functionalities
mentionned in my previous topic posted some days ago. This shell
should:
1- execute input commands from standard input, and also from a file
conatining the commands
2- does the redirection of the input and output from and to files.
3- retrieve the environment variables like HOME,..
>
I did the first question and it works fine, however when I modified the
code to implement the redirection it does not work anymore.
>
Could you please have a look to my code and tell me which is wrong
with?
>
I would also appreciate if you can tell me how to implement the 2nd
question in the function parse_line (see the code below):
>
Thank you very much for your help on this
>
Souissipro:
>
---------------SHELL---------------------------
#include<stdio. h>
#include<stdlib .h>
#include<string .h>
#include<unistd .h>
#include<fcntl. h>

<SNIP rest of UNIX specific code>

We only deal with standard C here ; you
need to ask your question at comp.unix.progr ammer

[Ed Prochak]

Re: Redirection issue


souissipro wrote:

Hi,
>
I have written a C program that does some of the functionalities
mentionned in my previous topic posted some days ago. This shell
should:
1- execute input commands from standard input, and also from a file
conatining the commands
2- does the redirection of the input and output from and to files.
3- retrieve the environment variables like HOME,..
>
I did the first question and it works fine, however when I modified the
code to implement the redirection it does not work anymore.
>
Could you please have a look to my code and tell me which is wrong
with?
>
I would also appreciate if you can tell me how to implement the 2nd
question in the function parse_line (see the code below):
>
Thank you very much for your help on this
>

Rather then us looking at your code, I have some general suggestions to
help you help yourself.

First: describe the problem.
the phrase "it does not work anymore" carries very little meaning.
"does not work" could be anything from not compiling, not executing,
crashing immediately, crashing the entire computer, and many other ways
of "not working". You have a case of berfore and after. What did it do
before that it does not do now? BE SPECIFIC. Properly defining the
problem often leads to finding the right solution.

Second: isolate the changes.
Run a diff on the current and previous versions of the program (You do
use a source control system don't you?) Determine how the changes could
have broken the code that was working. For example, look for global
variables with the same name as local variables. Watch for unterminated
block comments. Other common errors.

Three: trace your steps.
If you cannot figure it out from those ideas, try adding some debug
logging through the program OR run the program under a debugger.

Four: slash and burn.
If the problem is serious (e.g., crash and core dump) and nothing else
helps you isolate it, then start removing parts of code from the
current version until it no longer crashes, or you are back to the
previous working version. Cutting the body of functions, leaving only a
return, for example, may be useful.

You have a very good chance of finding the cause of this problem
yourself since you went from a working version to a non-working
version. You'll be a better programmer by learning how to find this
yourself. Just remember, the program is doing exactly what you told it
to do.

ed

[souissipro]

Re: Redirection issue


Ed Prochak wrote:

souissipro wrote:

Hi,

I have written a C program that does some of the functionalities
mentionned in my previous topic posted some days ago. This shell
should:
1- execute input commands from standard input, and also from a file
conatining the commands
2- does the redirection of the input and output from and to files.
3- retrieve the environment variables like HOME,..

I did the first question and it works fine, however when I modified the
code to implement the redirection it does not work anymore.

Could you please have a look to my code and tell me which is wrong
with?

I would also appreciate if you can tell me how to implement the 2nd
question in the function parse_line (see the code below):

Thank you very much for your help on this

>
Rather then us looking at your code, I have some general suggestions to
help you help yourself.
>
First: describe the problem.
the phrase "it does not work anymore" carries very little meaning.
"does not work" could be anything from not compiling, not executing,
crashing immediately, crashing the entire computer, and many other ways
of "not working". You have a case of berfore and after. What did it do
before that it does not do now? BE SPECIFIC. Properly defining the
problem often leads to finding the right solution.
>
Second: isolate the changes.
Run a diff on the current and previous versions of the program (You do
use a source control system don't you?) Determine how the changes could
have broken the code that was working. For example, look for global
variables with the same name as local variables. Watch for unterminated
block comments. Other common errors.
>
Three: trace your steps.
If you cannot figure it out from those ideas, try adding some debug
logging through the program OR run the program under a debugger.
>
Four: slash and burn.
If the problem is serious (e.g., crash and core dump) and nothing else
helps you isolate it, then start removing parts of code from the
current version until it no longer crashes, or you are back to the
previous working version. Cutting the body of functions, leaving only a
return, for example, may be useful.
>
You have a very good chance of finding the cause of this problem
yourself since you went from a working version to a non-working
version. You'll be a better programmer by learning how to find this
yourself. Just remember, the program is doing exactly what you told it
to do.
>
ed

Hi ed,

Thanks for your comments. I would add the following information in
order to make things more clear:
In my program, I first implented the three functions: parse_line,
simple_cmd and the main in order to execute input commands from
standard input, and also from input file. This works fine which means
after compilation and execution of the shell with a simple command like
"ls" or "pwd" I get the expected result. When I pass a file as input
all the command in the file are executed as well.

Now, I modified the function parse_line in order to manage the
input/output. I ceatred a new function redir_cmd that receives the name
of files for input and output. I did a modification in the main:
if argc==1 this means simple command;
if argc==2 this means command from file and output STDINOUT;
if argc==3 input from a file an dthe output directed to another file
instead of STDINOUT.
Please have a look to the shell for more details.

The new shell compiles fine but at the execution it does not execute
the commands of the input file and hence it does not make the output in
the output file.

My question is then could you please have a look to my shell and tell
me which is wrong there. I'm not expert in C and I need concrete help
on that.

Thanks in advance.

Souipro

[Barry Margolin]

Re: Redirection issue

In article <1156959022.965 599.76790@e3g20 00cwe.googlegro ups.com>,
"souissipro " <souissipro@yah oo.frwrote:

int
redir_cmd(char *argv[],char *in, char *out)

You don't seem to handle the case where only on direction is being
redirected. Perhaps you should allow a null point to be passed for the
ones that aren't being redirected, and check for this below before
calling open() and dup2().

This would also allow you to avoid having separate simple_cmd() and
redir_cmd() functions -- a simple command is just what you get if both
in and out are null, because it won't redirect anything.

{
int i;
int pid, status;
int fd1, fd2;
int dummy;
>
if (strcmp(argv[0],"exit")==0)
return do_exit(argv);
for(i=0;argv[i]!=NULL;i++)
printf("argv[%i]=\"%s\";\n",i,a rgv[i]);
>
if (strcmp(argv[0],"cd")==0)
{
return chdir(*argv);
}
>
if (fork() == 0) {
fd1 = open(in, O_RDONLY);
if (fd1 < 0) {
perror("can't open file : in");
exit(1);
}
>
if (dup2(fd1, out) != 0) {

This should be:

if (dup2(fd1, STDIN_FILENO) != 0) {

perror("catf1f2 : dup2(in, 0)");
exit(1);
}
close(fd1);
>
fd2 = open(out, O_WRONLY | O_TRUNC | O_CREAT, 0644);
if (fd2 < 0) {
perror("catinou t: out");
exit(2);
}
/*
if (dup2(fd2, 1) != 1) {

It's preferable to use STDOUT_FILENO instead of hard-coding 1.

perror("catf1f2 : dup2(out, 1)");
exit(1);
}
*/
close(fd2);
>
execvp(*argv, argv);
perror(*argv);
exit(1);
>
}
>
/*
* parent executes wait.
*/
else {
wait(&dummy);
}
>
}

--
Barry Margolin, barmar@alum.mit .edu
Arlington, MA
*** PLEASE post questions in newsgroups, not directly to me ***
*** PLEASE don't copy me on replies, I'll read them in the group ***

[Ed Prochak]

Re: Redirection issue


souissipro wrote:

Ed Prochak wrote:

souissipro wrote:

Hi,
>
I have written a C program that does some of the functionalities
mentionned in my previous topic posted some days ago. This shell
should:
1- execute input commands from standard input, and also from a file
conatining the commands
2- does the redirection of the input and output from and to files.
3- retrieve the environment variables like HOME,..
>
I did the first question and it works fine, however when I modified the
code to implement the redirection it does not work anymore.
>
Could you please have a look to my code and tell me which is wrong
with?
>
I would also appreciate if you can tell me how to implement the 2nd
question in the function parse_line (see the code below):
>
Thank you very much for your help on this
>

Rather then us looking at your code, I have some general suggestions to
help you help yourself.

First: describe the problem.
the phrase "it does not work anymore" carries very little meaning.
"does not work" could be anything from not compiling, not executing,
crashing immediately, crashing the entire computer, and many other ways
of "not working". You have a case of berfore and after. What did it do
before that it does not do now? BE SPECIFIC. Properly defining the
problem often leads to finding the right solution.

Second: isolate the changes.
Run a diff on the current and previous versions of the program (You do
use a source control system don't you?) Determine how the changes could
have broken the code that was working. For example, look for global
variables with the same name as local variables. Watch for unterminated
block comments. Other common errors.

Three: trace your steps.
If you cannot figure it out from those ideas, try adding some debug
logging through the program OR run the program under a debugger.

Four: slash and burn.
If the problem is serious (e.g., crash and core dump) and nothing else
helps you isolate it, then start removing parts of code from the
current version until it no longer crashes, or you are back to the
previous working version. Cutting the body of functions, leaving only a
return, for example, may be useful.

You have a very good chance of finding the cause of this problem
yourself since you went from a working version to a non-working
version. You'll be a better programmer by learning how to find this
yourself. Just remember, the program is doing exactly what you told it
to do.

ed

>
>
Hi ed,
>
Thanks for your comments. I would add the following information in
order to make things more clear:
In my program, I first implented the three functions: parse_line,
simple_cmd and the main in order to execute input commands from
standard input, and also from input file. This works fine which means
after compilation and execution of the shell with a simple command like
"ls" or "pwd" I get the expected result. When I pass a file as input
all the command in the file are executed as well.
>
Now, I modified the function parse_line in order to manage the
input/output. I ceatred a new function redir_cmd that receives the name
of files for input and output. I did a modification in the main:
if argc==1 this means simple command;
if argc==2 this means command from file and output STDINOUT;
if argc==3 input from a file an dthe output directed to another file
instead of STDINOUT.
Please have a look to the shell for more details.
>
The new shell compiles fine but at the execution it does not execute
the commands of the input file and hence it does not make the output in
the output file.
>
My question is then could you please have a look to my shell and tell
me which is wrong there. I'm not expert in C and I need concrete help
on that.
>
Thanks in advance.
>
Souipro

My question is did you follow ANY of the suggestions I gave you? Your
error is very simple. I did look at your code and found the problem
right away. I really am trying to help you learn. This problem is
independent of the programming language. So please, review your own
code. before you scroll down to what I see as the root problem.

Hint, compare what you describe the program doing above with how the
program is actually coded.









DON'T PEEK!









Final suggestion before revealing the answer: Take a printout of the
program, go to a friend (even a nonprogrammer) and describe to them
what each part of the program does. If that doesn't work, then look
below














You never called the new function!

BTW, long term your approach is going about it the hard way, but you'll
learn more this way.
Ed

[Bo Yang]

Re: Redirection issue

>
int
redir_cmd(char *argv[],char *in, char *out)
{
int i;
int pid, status;
int fd1, fd2;
int dummy;
>
if (strcmp(argv[0],"exit")==0)
return do_exit(argv);
for(i=0;argv[i]!=NULL;i++)
printf("argv[%i]=\"%s\";\n",i,a rgv[i]);
>
if (strcmp(argv[0],"cd")==0)
{
return chdir(*argv);
}
>
if (fork() == 0) {
fd1 = open(in, O_RDONLY);
if (fd1 < 0) {
perror("can't open file : in");
exit(1);
}
>
if (dup2(fd1, out) != 0) {

You dup the out to fd1 , but out is a char pointer ,
what do you want to do ?
This must to be failed .

You can just dup the stdin to in , and dup stdout to out .

[souissipro]

Re: Redirection issue


Bo Yang wrote:

int
redir_cmd(char *argv[],char *in, char *out)
{
int i;
int pid, status;
int fd1, fd2;
int dummy;

if (strcmp(argv[0],"exit")==0)
return do_exit(argv);
for(i=0;argv[i]!=NULL;i++)
printf("argv[%i]=\"%s\";\n",i,a rgv[i]);

if (strcmp(argv[0],"cd")==0)
{
return chdir(*argv);
}

if (fork() == 0) {
fd1 = open(in, O_RDONLY);
if (fd1 < 0) {
perror("can't open file : in");
exit(1);
}

if (dup2(fd1, out) != 0) {

You dup the out to fd1 , but out is a char pointer ,
what do you want to do ?
This must to be failed .
>
You can just dup the stdin to in , and dup stdout to out .

Hi Bo,

I have changed the fd1 and fd2 types and the redirection is now
working. The shell compiles . The commands of the input file
(input.txt) have been executed and the output is sent to the outout
file (ouput.txt).
Now, I have another issues in the sens that the simple command and the
commands passed in a file are not executed.
In the main function I have treated 3 cases : simple command, input
file, and redirection. But the problem may be in the rediretion
(redirect_cmd) function.

Any idea please.

Thanks,

Souissipro

[souissipro]

Re: Redirection issue


Barry Margolin wrote:

In article <1156959022.965 599.76790@e3g20 00cwe.googlegro ups.com>,
"souissipro " <souissipro@yah oo.frwrote:
>

int
redir_cmd(char *argv[],char *in, char *out)

>
You don't seem to handle the case where only on direction is being
redirected. Perhaps you should allow a null point to be passed for the
ones that aren't being redirected, and check for this below before
calling open() and dup2().
>
This would also allow you to avoid having separate simple_cmd() and
redir_cmd() functions -- a simple command is just what you get if both
in and out are null, because it won't redirect anything.
>

{
int i;
int pid, status;
int fd1, fd2;
int dummy;

if (strcmp(argv[0],"exit")==0)
return do_exit(argv);
for(i=0;argv[i]!=NULL;i++)
printf("argv[%i]=\"%s\";\n",i,a rgv[i]);

if (strcmp(argv[0],"cd")==0)
{
return chdir(*argv);
}

if (fork() == 0) {
fd1 = open(in, O_RDONLY);
if (fd1 < 0) {
perror("can't open file : in");
exit(1);
}

if (dup2(fd1, out) != 0) {

>
This should be:
>
if (dup2(fd1, STDIN_FILENO) != 0) {
>

perror("catf1f2 : dup2(in, 0)");
exit(1);
}
close(fd1);

fd2 = open(out, O_WRONLY | O_TRUNC | O_CREAT, 0644);
if (fd2 < 0) {
perror("catinou t: out");
exit(2);
}
/*
if (dup2(fd2, 1) != 1) {

>
It's preferable to use STDOUT_FILENO instead of hard-coding 1.
>

perror("catf1f2 : dup2(out, 1)");
exit(1);
}
*/
close(fd2);

execvp(*argv, argv);
perror(*argv);
exit(1);

}

/*
* parent executes wait.
*/
else {
wait(&dummy);
}

}

>
--
Barry Margolin, barmar@alum.mit .edu
Arlington, MA
*** PLEASE post questions in newsgroups, not directly to me ***
*** PLEASE don't copy me on replies, I'll read them in the group ***

Hi Barry ,

Thanks for your comments.

I have changed the fd1 and fd2 types and also 0 and 1 by STDOUT_FILENO
and STDIN_FILENO. The redirection is now working. The shell compiles .
The commands of the input file (input.txt) have been executed and the
output is sent to the outout file (ouput.txt).
Now, I have another issues in the sens that the simple command and the
commands passed in a file are not executed.
In the main function I have treated 3 cases : simple command, input
file, and redirection. But the problem may be in the rediretion
(redirect_cmd) function and as you said the code is not handle the case
where only on direction is being
redirected.
Could you please clarify this by some lines of codes in this fucntion.


Thanks,

Souissipro

[souissipro]

Re: Redirection issue


Ed Prochak wrote:

souissipro wrote:

Ed Prochak wrote:

souissipro wrote:
Hi,

I have written a C program that does some of the functionalities
mentionned in my previous topic posted some days ago. This shell
should:
1- execute input commands from standard input, and also from a file
conatining the commands
2- does the redirection of the input and output from and to files.
3- retrieve the environment variables like HOME,..

I did the first question and it works fine, however when I modified the
code to implement the redirection it does not work anymore.

Could you please have a look to my code and tell me which is wrong
with?

I would also appreciate if you can tell me how to implement the 2nd
question in the function parse_line (see the code below):

Thank you very much for your help on this

>
Rather then us looking at your code, I have some general suggestions to
help you help yourself.
>
First: describe the problem.
the phrase "it does not work anymore" carries very little meaning.
"does not work" could be anything from not compiling, not executing,
crashing immediately, crashing the entire computer, and many other ways
of "not working". You have a case of berfore and after. What did it do
before that it does not do now? BE SPECIFIC. Properly defining the
problem often leads to finding the right solution.
>
Second: isolate the changes.
Run a diff on the current and previous versions of the program (You do
use a source control system don't you?) Determine how the changes could
have broken the code that was working. For example, look for global
variables with the same name as local variables. Watch for unterminated
block comments. Other common errors.
>
Three: trace your steps.
If you cannot figure it out from those ideas, try adding some debug
logging through the program OR run the program under a debugger.
>
Four: slash and burn.
If the problem is serious (e.g., crash and core dump) and nothing else
helps you isolate it, then start removing parts of code from the
current version until it no longer crashes, or you are back to the
previous working version. Cutting the body of functions, leaving only a
return, for example, may be useful.
>
You have a very good chance of finding the cause of this problem
yourself since you went from a working version to a non-working
version. You'll be a better programmer by learning how to find this
yourself. Just remember, the program is doing exactly what you told it
to do.
>
ed

Hi ed,

Thanks for your comments. I would add the following information in
order to make things more clear:
In my program, I first implented the three functions: parse_line,
simple_cmd and the main in order to execute input commands from
standard input, and also from input file. This works fine which means
after compilation and execution of the shell with a simple command like
"ls" or "pwd" I get the expected result. When I pass a file as input
all the command in the file are executed as well.

Now, I modified the function parse_line in order to manage the
input/output. I ceatred a new function redir_cmd that receives the name
of files for input and output. I did a modification in the main:
if argc==1 this means simple command;
if argc==2 this means command from file and output STDINOUT;
if argc==3 input from a file an dthe output directed to another file
instead of STDINOUT.
Please have a look to the shell for more details.

The new shell compiles fine but at the execution it does not execute
the commands of the input file and hence it does not make the output in
the output file.

My question is then could you please have a look to my shell and tell
me which is wrong there. I'm not expert in C and I need concrete help
on that.

Thanks in advance.

Souipro

>
My question is did you follow ANY of the suggestions I gave you? Your
error is very simple. I did look at your code and found the problem
right away. I really am trying to help you learn. This problem is
independent of the programming language. So please, review your own
code. before you scroll down to what I see as the root problem.
>
Hint, compare what you describe the program doing above with how the
program is actually coded.
>
>
>
>
>
>
>
>
>
DON'T PEEK!
>
>
>
>
>
>
>
>
>
Final suggestion before revealing the answer: Take a printout of the
program, go to a friend (even a nonprogrammer) and describe to them
what each part of the program does. If that doesn't work, then look
below
>
>
>
>
>
>
>
>
>
>
>
>
>
>
You never called the new function!
>
BTW, long term your approach is going about it the hard way, but you'll
learn more this way.
Ed

Hi Ed,

Thank you anyway for your comments. I appreciate them very much.
I decommented the code in the new function redir_cmd, changed the types
of fd1 and fd2 to char and the redirection works fine now. However, the
simple command and the simple input file are now not working.
Please any precise idea because I have to finish this shell for
tomorrow.

Thanks,
souissipro

[souissipro]

Re: Redirection issue


Ed Prochak wrote:

souissipro wrote:

Ed Prochak wrote:

souissipro wrote:
Hi,

I have written a C program that does some of the functionalities
mentionned in my previous topic posted some days ago. This shell
should:
1- execute input commands from standard input, and also from a file
conatining the commands
2- does the redirection of the input and output from and to files.
3- retrieve the environment variables like HOME,..

I did the first question and it works fine, however when I modified the
code to implement the redirection it does not work anymore.

Could you please have a look to my code and tell me which is wrong
with?

I would also appreciate if you can tell me how to implement the 2nd
question in the function parse_line (see the code below):

Thank you very much for your help on this

>
Rather then us looking at your code, I have some general suggestions to
help you help yourself.
>
First: describe the problem.
the phrase "it does not work anymore" carries very little meaning.
"does not work" could be anything from not compiling, not executing,
crashing immediately, crashing the entire computer, and many other ways
of "not working". You have a case of berfore and after. What did it do
before that it does not do now? BE SPECIFIC. Properly defining the
problem often leads to finding the right solution.
>
Second: isolate the changes.
Run a diff on the current and previous versions of the program (You do
use a source control system don't you?) Determine how the changes could
have broken the code that was working. For example, look for global
variables with the same name as local variables. Watch for unterminated
block comments. Other common errors.
>
Three: trace your steps.
If you cannot figure it out from those ideas, try adding some debug
logging through the program OR run the program under a debugger.
>
Four: slash and burn.
If the problem is serious (e.g., crash and core dump) and nothing else
helps you isolate it, then start removing parts of code from the
current version until it no longer crashes, or you are back to the
previous working version. Cutting the body of functions, leaving only a
return, for example, may be useful.
>
You have a very good chance of finding the cause of this problem
yourself since you went from a working version to a non-working
version. You'll be a better programmer by learning how to find this
yourself. Just remember, the program is doing exactly what you told it
to do.
>
ed

Hi ed,

Thanks for your comments. I would add the following information in
order to make things more clear:
In my program, I first implented the three functions: parse_line,
simple_cmd and the main in order to execute input commands from
standard input, and also from input file. This works fine which means
after compilation and execution of the shell with a simple command like
"ls" or "pwd" I get the expected result. When I pass a file as input
all the command in the file are executed as well.

Now, I modified the function parse_line in order to manage the
input/output. I ceatred a new function redir_cmd that receives the name
of files for input and output. I did a modification in the main:
if argc==1 this means simple command;
if argc==2 this means command from file and output STDINOUT;
if argc==3 input from a file an dthe output directed to another file
instead of STDINOUT.
Please have a look to the shell for more details.

The new shell compiles fine but at the execution it does not execute
the commands of the input file and hence it does not make the output in
the output file.

My question is then could you please have a look to my shell and tell
me which is wrong there. I'm not expert in C and I need concrete help
on that.

Thanks in advance.

Souipro

>
My question is did you follow ANY of the suggestions I gave you? Your
error is very simple. I did look at your code and found the problem
right away. I really am trying to help you learn. This problem is
independent of the programming language. So please, review your own
code. before you scroll down to what I see as the root problem.
>
Hint, compare what you describe the program doing above with how the
program is actually coded.
>
>
>
>
>
>
>
>
>
DON'T PEEK!
>
>
>
>
>
>
>
>
>
Final suggestion before revealing the answer: Take a printout of the
program, go to a friend (even a nonprogrammer) and describe to them
what each part of the program does. If that doesn't work, then look
below
>
>
>
>
>
>
>
>
>
>
>
>
>
>
You never called the new function!
>
BTW, long term your approach is going about it the hard way, but you'll
learn more this way.
Ed

Hi Ed,

Thank you anyway for your comments. I appreciate them very much.
I decommented the code in the new function redir_cmd, changed the types
of fd1 and fd2 to char and the redirection works fine now. However, the
simple command and the simple input file are now not working.
Please any precise idea because I have to finish this shell for
tomorrow.

Thanks,
souissipro

[Ed Prochak]

Re: Redirection issue

souissipro wrote:

Ed Prochak wrote:

[snip]

My question is did you follow ANY of the suggestions I gave you? Your
error is very simple. I did look at your code and found the problem
right away. I really am trying to help you learn. This problem is
independent of the programming language. So please, review your own
code. before you scroll down to what I see as the root problem.

[snip]

You never called the new function!

BTW, long term your approach is going about it the hard way, but you'll
learn more this way.
Ed

>
Hi Ed,
>
Thank you anyway for your comments. I appreciate them very much.
I decommented the code in the new function redir_cmd, changed the types
of fd1 and fd2 to char and the redirection works fine now. However, the
simple command and the simple input file are now not working.
Please any precise idea because I have to finish this shell for
tomorrow.
>
Thanks,
souissipro

based on your reply, I take it that
you did NOT follow any of my suggestions. (I could be wrong. 8^)
your source listing from the first post is not the version of code you
are currrently discussing since my comment was about the main() routine
and nothing was commented out in that version. I haven't seen you post
another version.

Finally, NEVER expect responses in a newsgroup in less than 24 hours.
Some people will not even have seen your request before a full day has
past. Just remember, newsgroups are not like AOL Chat rooms.

Good luck, you will need it.
Ed

[souissipro]

Re: Redirection issue


Ed Prochak wrote:

souissipro wrote:

Ed Prochak wrote:

[snip]

>
My question is did you follow ANY of the suggestions I gave you? Your
error is very simple. I did look at your code and found the problem
right away. I really am trying to help you learn. This problem is
independent of the programming language. So please, review your own
code. before you scroll down to what I see as the root problem.
>

[snip]

>
You never called the new function!
>
BTW, long term your approach is going about it the hard way, but you'll
learn more this way.
Ed

Hi Ed,

Thank you anyway for your comments. I appreciate them very much.
I decommented the code in the new function redir_cmd, changed the types
of fd1 and fd2 to char and the redirection works fine now. However, the
simple command and the simple input file are now not working.
Please any precise idea because I have to finish this shell for
tomorrow.

Thanks,
souissipro

>
based on your reply, I take it that
you did NOT follow any of my suggestions. (I could be wrong. 8^)
your source listing from the first post is not the version of code you
are currrently discussing since my comment was about the main() routine
and nothing was commented out in that version. I haven't seen you post
another version.
>
Finally, NEVER expect responses in a newsgroup in less than 24 hours.
Some people will not even have seen your request before a full day has
past. Just remember, newsgroups are not like AOL Chat rooms.
>
Good luck, you will need it.
Ed

ED,

OK!
to reply your answer. The new function is called in the main here:
if (argc == 3) {
f = fopen(argv[1], "r");
if (f == NULL) {
fprintf(stderr, "impossible d'ouvrir le fichier en lecture\n");
exit(EXIT_FAILU RE);
}
/* fgets fait une lecture ligne par ligne, et stocke */
/* la ligne lue (y compris le \n) dans buf */
/* il faut que la ligne fasse moins de 255 caracteres */
/* sinon seuls les 255 premiers caracteres sont lus */

while (fgets(buf, 255, f) != NULL) {
// call of the new function via parse_line function
parse_line(buf, argv[1],argv[2]);
printf("%s",buf );
}
isn't right?

[Dave Thompson]

Re: Redirection issue

On 30 Aug 2006 10:30:23 -0700, "souissipro " <souissipro@yah oo.fr>
wrote:

Hi,
>
I have written a C program that does some of the functionalities
mentionned in my previous topic posted some days ago. This shell
should:
1- execute input commands from standard input, and also from a file
conatining the commands

Clarification: according to your description downthread and your code,
the program has the generic or abstract ability to execute commands
read from stdin and the ability to execute commands read from a file,
but on a given run it does only one or the other not both.

2- does the redirection of the input and output from and to files.
3- retrieve the environment variables like HOME,..
>
I did the first question and it works fine, however when I modified the
code to implement the redirection it does not work anymore.
>

As already said, the parts of this code about creating child processes
by fork and exec* (and wait), and managing Unix-level fd's (open,
dup2, close) as is needed for redirection, are not part of standard C
and thus offtopic here and belong in comp.unix.progr ammer or similar.
But I will make some comments anyway.

int
do_exit(char *argv[])
{
if (argv[1]==NULL)
exit(EXIT_SUCCE SS);
else
fprintf(stderr, "Error: don't take arguments.\n");
return 1;
}
>

In conventional Unix shells "exit" does take an optional argument. I
assume your teacher does not want that feature.

int
simple_cmd(char *argv[])
{
int i;
int pid, status;
>
if (strcmp(argv[0],"exit")==0)
return do_exit(argv);
for(i=0;argv[i]!=NULL;i++)
printf("argv[%i]=\"%s\";\n",i,a rgv[i]);
>
if (strcmp(argv[0],"cd")==0)
{
return chdir(*argv);
}
>

First, this uses the command name "cd" as the new directory name,
which is never correct except by a very unlikely coincidence. You want
argv[1]. But only if it is non-NULL. If it is NULL -- if "cd" is given
with no argument(s) -- you might treat it specially; conventional Unix
shells in this case revert to the user's home directory.

Second, if chdir() fails, as it will with the bug above, you don't
display an error message as you do in other cases; you do return an
error code, but your calling code ignores it, so the user isn't told.

<snip>

int
redir_cmd(char *argv[],char *in, char *out)
{

<snip mostly same as above,
plus dup2 usage already corrected by Barry>

Also, you have no provision for redirecting STDERR (fd 2).
Conventional shells do, and this is often needed and used.
Some (most?) of them can also explicitly manage, and redirect, fds
higher than 2, but that is rarely needed or used.

int
parse_line(char *s,char *in, char *out)
{
char *argv[NARGS], *cur, *old;
int i=0;
>
if ((s[0]=='#')||(s[0]=='\n'))
return 0;
old=cur=s;
while((cur=strp brk(cur,sep))!= NULL) {
argv[i++]=old;
cur[0]='\0';
cur++;
old=cur;
}
argv[i]=NULL;

You don't check for overflowing your argv[] array which will occur if
an input line has more than NARGS-1 (49) tokens.

You resume parsing after exactly one separator character; if a user
gives "foo bar" (with two spaces) you will treat this as "foo"
<empty"bar". Conventional shells allow multiple spaces (and also
other whitespace characters like TAB and VT, though rarely used).

if
return redir_cmd(argv, in, out);
}

This is a syntax error and can't be code you actually compiled.
I guess you had (or want) something like
if in and out don't specify redirection (how?)
call simple_cmd (and return its value, though not used)
else (they do specify redirection) call redir_cmd (ditto)
although as Barry already said there is much commonality between the
simple_cmd and redir_cmd cases; you are probably better off writing
one routine which handles both cases but does the redirection only
conditionally (when specified by the parameters).

>
/* Main function
*/
>
int
main(int argc, char *argv[])
{
char s[MAXL];
>
int i;
char buf[255];
FILE *f;
>

You never use s. You do use buf to contain a line, and the name MAXL
implies that it is intended to specify the maximum length of a line.

if (argc 3) {
fprintf(stderr, "incorrect arguments \n");
exit(EXIT_FAILU RE);

Good for using the standard (and ontopic) EXIT_FAILURE (and _SUCCESS)
rather than specific (and often arbitrary) int values.

}
if (argc == 2) {
f = fopen(argv[1], "r");
if (f == NULL) {
fprintf(stderr, "impossible to open the file for reading\n");

You might better use perror, or include strerror(errno) . Standard C
does not guarantee that fopen() failure sets a useful value in errno,
but most if not all systems that have fork() and exec() do.

exit(EXIT_FAILU RE);
}
>
while (fgets(buf, 255, f) != NULL) {

Instead of using the 'magic number' 255, which will become wrong if
you change your declaration of buf above, use sizeof(buf) or just
sizeof buf (without the parentheses) if you like; or if you meant to
use MAXL to declare the buffer you can also use MAXL here.

parse_line(buf, argv[1],"stdout");
printf("%s",buf );

Since parse_line stored null terminators into buf, this will print
only the first token from the input line (unless a comment).
And it's not good shell behavior to echo commands anyway.

}
fclose(f);
}

This logic will result in each input line that is a command reading as
its standard input the same file as the shell _starting from the
beginning_; i.e. if your input file is:
someprog some args
otherprog different arguments
then someprog will read as its input the lines
someprog some args
otherprog different arguments
and then so will otherprog; if you try to put data in like
someprog some args
data for someprog
more data
otherprog different arguments
then your shell will try to execute 'data for someprog' as a command
with results almost certainly different from what was desired. This is
why "sh <foo" is rarely useful and "sh -c foo" is quite different, and
"command <<DELIMITER" doesn't use a filename at all.

Depending on how parse_line and redir_cmd work, this may also replace
the entire file "stdout" from the beginning for each command, so only
the output from the last command remains. This also is rarely useful,
and not the conventional default, which is to share the fd and thus
combine the output though not always in the expected order.

if (argc == 3) {

<snip similar to above>

if (argc == 1) {

<snip similar to above, except that depending on parse_line and
redir_cmd each command may read "stdin" from the beginning and replace
"stdout" leaving only the last output.


- David.Thompson1 at worldnet.att.ne t