Re: Redirection issue

On 30 Aug 2006 10:30:23 -0700, "souissipro" <souissipro@xxxxxxxx>
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.programmer or similar.
But I will make some comments anyway.


int
do_exit(char *argv[])
{
if (argv[1]==NULL)
exit(EXIT_SUCCESS);
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,argv[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=strpbrk(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_FAILURE);

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

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.net
.

Relevant Pages

  • Re: Redirection issue
    ... 1- execute input commands from standard input, ... the phrase "it does not work anymore" carries very little meaning. ... after compilation and execution of the shell with a simple command like ...
    (comp.lang.c)
  • Re: Redirection issue
    ... 1- execute input commands from standard input, ... "does not work" could be anything from not compiling, not executing, ... after compilation and execution of the shell with a simple command like ...
    (comp.lang.c)
  • vulnerabilities in scponly
    ... without allowing shell access. ... scponly makes no effort to verify the path to the scp or sftp-server ... arbitrary commands by simply uploading a file. ... However, if this is *NOT* the case, the user could execute arbitrary ...
    (Bugtraq)
  • Re: how to make function known to subshell
    ... In my .profile I source a file with a my shell functions, ... When I open a new ksh shell or execute a shell script, ... > must resort to aliases and functions. ... builtin commands have a higher ...
    (comp.unix.shell)
  • Re: Help needed with dd command
    ... output and set them each to be input to the cat. ... says the shell had something else open for 2 and 3 before then. ... It sets up a race condition: Commands in parens or back ... Arrow redirection happens the "the" command is started. ...
    (comp.unix.admin)