0

Hello and thank you for attention. I am writing my own shell and I have small problem with redirect output to file. For example user wrote: ls -l >> output. If I find ">>", I should redirect first part of command, I mean call effect ls -l to file "output". I try to do it in case 1 but to file is redirected just one lane and the program is stopped, there is not appear "Shell -> " and nothing is going on. Can you give some advice to solve that problem? Thanks.

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <signal.h>
#include <string.h>
#include <sys/wait.h>
#include <sys/types.h>
#include <ctype.h>
#include <errno.h>
#include <sys/stat.h>
#include <fcntl.h>

int *parse(char *linia, char **argv)
{
    int counter = -1;
    while (*linia != '\0') 
    {       
        while (*linia == ' ' || *linia == '\t' || *linia == '\n')
            *linia++ = '\0';  
        *argv++ = linia; 
        counter++;   
        while (*linia != '\0' && *linia != ' ' &&  *linia != '\t' && *linia != '\n') 
            linia++;            
    }
    *argv = '\0'; 
    return counter;
}

void execute(char **argv)
{
    pid_t pid;
    int status;

    if ((pid = fork()) < 0) 
    {     
        printf("*** ERROR ***\n");
        exit(1);
    }
    else if (pid == 0) 
    {          
        if (execvp(*argv, argv) < 0) 
        {    
            printf("*** ERROR ***\n");
            exit(1);
        }
    }
    else 
    {                                  
        while (wait(&status) != pid);
    }
}                


int specialChar(char *argv)
{
    int i=0;
    while(argv[i]!='\0')
    {
        if(argv[i]=='>' && argv[i+1]=='>')
            return 1;           
        else if(argv[i]=='&')
            return 2;           
        else if(argv[i]=='|')
            return 3;           
        i++;
    }
}

void  main()
{
    char command[20];
    char *argv[64];
    char **history = (char**)malloc(20*sizeof(char*));
    int counter1=-1;
    int counter2=0;
    for(counter2 = 0; counter2<20; counter2++)
    {
        history[counter2]=(char*)malloc(100*sizeof(char));
    }
    int start = 0;
    FILE *file;
    file=fopen("his", "w");
    if(!file)
        printf("ERROR"); 
    int i=0;
    while (1) 
    {       
        printf("Shell -> ");     
        gets(command);
        counter1++;
        strcpy(history[counter1],command);
        fopen("his", "w");
        if(counter1<20)
            for(i=0; i<=counter1; i++)
            {
                fprintf(file,"%s\n",history[i]);
            }    
        else
            for(i=counter1-20; i<counter1; i++)
            {
                fprintf(file,"%s\n",history[i]);
            }
        fflush(file);
        printf("\n"); 
        switch(specialChar(command))
        {       
            case 1:
            i = parse(command, argv);
            int file1 = open(argv[i], O_APPEND | O_WRONLY);
            dup2(file1,1)   ;  
            if (strcmp(argv[0], "exit") == 0) 
                  exit(0);       
            execute(argv); 
            close(file1);
            break;
            case 2:

            break;
            case 3:

            break;
            default:
            parse(command, argv);      
            if (strcmp(argv[0], "exit") == 0) 
                  exit(0);       
            execute(argv);  
            break;     
        }
        fclose(file);
        }
}
ByQ
  • 37
  • 6
  • That's because you are redirecting the shell's output to the file "output", so obviously when you write "Shell ->", it goes to the file "output" and not to the screen. – user253751 Apr 15 '17 at 11:16
  • I don t think so. I open and close file after printf("Shell ->") – ByQ Apr 15 '17 at 11:24
  • Irrelevant. You close one handle to the file which is not the standard output handle. (But if that did actually close the file, which it doesn't, that would just mean you got errors when writing to the now-closed file and nothing would still appear on the screen) – user253751 Apr 15 '17 at 11:25
  • Closing the file is irrelevant. – user253751 Apr 15 '17 at 11:40
  • So how can I fix that problem? I checked that without closeing that file result is that same. – ByQ Apr 15 '17 at 11:44
  • You could redirect the new process's output instead of the shell's output. And by that I mean use the exact same code (which redirects the current process's output) but run it in the new process instead of in the shell. – user253751 Apr 15 '17 at 11:52
  • I should exit of that file. I get also communicate that ls haven t got acces to '>>'. There is no catalog or file. – ByQ Apr 15 '17 at 11:55
  • 1
    Never use gets: http://stackoverflow.com/questions/1694036/why-is-the-gets-function-so-dangerous-that-it-should-not-be-used – William Pursell Apr 15 '17 at 12:40
  • although VS will allow you to get away with mis-writing several things, there are only a couple of valid ways to write the signature for `main()` They are: `int main( void )` and `int main( int argc, char* argv[] )` Notice that all of them have a return type of `int`. – user3629249 Apr 16 '17 at 02:44
  • when calling any of the heap allocation functions: (malloc, calloc, realloc) 1) the returned type is `void*` so can be assigned to any other pointer. Casting just clutters the code, making it much more difficult to understand, debug, etc. Suggest removing the 'casting' 2) Always check (!=NULL) the returned value to assure the operation was successful. – user3629249 Apr 16 '17 at 02:46
  • for ease of readability and understanding: 1) separate code blocks (for, if, else, while, do...while, switch, case, default) via a single blank line. 2) it is a poor programming practice to have names/types only differ by capitalization I.E. `FILE` and `file`. Similar considerations for variable names that only differ by the last character 'counter1' and 'counter2` – user3629249 Apr 16 '17 at 02:51
  • the expression: `sizeof(char)` is defined in the standard as 1. multiplying anything by 1 has not effect and just clutters the code, making the code much more difficult to understand, debug, etc – user3629249 Apr 16 '17 at 02:52
  • a error message should be output to `stderr` not `stdout` and a failure to open a file is a reason to exit the code, no continue as if everything is all right. so this line: `printf("ERROR");` should be: `{ perror( "fopen to write is file failed" ); exit( EXIT_FAILURE ); }` Note, before calling `exit()` should also pass each of the pointers returned from `malloc()` to `free()` – user3629249 Apr 16 '17 at 02:56
  • the function: `gets()` has been depreciated and is completely missing from recent versions of the C language. Much better to use `fgets()` < which does have a different parameter list > – user3629249 Apr 16 '17 at 02:58
  • best to not be 'cute' by leaving off optional opening/closing braces '{' and '}' because that is a prime way to result in problems when the code is later being modified – user3629249 Apr 16 '17 at 02:59
  • when calling `fopen()`, always check (!=NULL) the returned value to assure the operation was successful – user3629249 Apr 16 '17 at 03:01
  • why open `his` for writing, before the `while()` loop, then open it again just inside the while()` loop without an intervening call to `fclose()` Note: opening a file with mode "w"` truncates the file, probably not what you want to do. Suggest "r+" or "a" – user3629249 Apr 16 '17 at 03:06
  • this kind of expression: `*argv++` will not do what your expecting due to the precedence of the C operators. Suggest: `(*argv)++ – user3629249 Apr 16 '17 at 03:20
  • the statement: `if (strcmp(argv[0], "exit") == 0)` will (most likely) not recognize the string: `exit` because of the trailing newline suggest: ``if (strncmp(argv[0], "exit", sizeof( "exit") ) == 0)` – user3629249 Apr 16 '17 at 03:24

0 Answers0