Correct handling of pointers

4

I'm having problems performing implementations of some of the shell commands in C. From my point of view it seems that it is due to an incorrect handling of pointers in C (I have not learned this language for a short time). At the moment I am trying to implement the directory change and export functions, although the directory change works when I write the path to the directory to which I want to move in the code itself. It does not do it when that path is entered by the user. I'm pretty sure it's because of that args[1] that happened as an argument. The same happens with the export, although in this case directly from segment violation. I tried to print args [1] as you will see in the code but it gives segment violation when doing that. I would appreciate someone helping me to learn how to do what I want to do correctly.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <unistd.h>
#define PROMPT "$"
#define MAX_LINE 512


int parse_args(char **args, char *line){
    int n=0;
    char* token;
    char delimit[]=" \t\r\n\v\f";
    token=strtok(line,delimit);
    while(token!=NULL){
        printf("token%i: %s\n",n,token);
        args=token;
        n++;
        args++;
        token=strtok(NULL,delimit);
    }
    printf("token%i: %s\n",n,token);
    args=token;
    return n;
}

char *read_line(char *line){
    printf("%s%s ",getenv("USER"),PROMPT);
    fflush(stdout);
    line=fgets(line,MAX_LINE,stdin);
    return line;
}


int execute_line(char *line){
    char **args;
    parse_args(args,line);
    check_internal(args);
    return 0;
}

int check_internal(char **args){
    if( strcmp(args, "cd")==0 ){
        internal_cd();
    } else{
        if( strcmp(args, "export")==0 ){
            internal_export();
        }else{
            if( strcmp(args, "source")==0 ){
                internal_source();
            }else{
                if( strcmp(args, "jobs")==0 ){
                    internal_jobs();
                }else{

                    return 0;
                }
            }
        }
    }
}

int internal_cd(char **args){
    char buff[50];
    printf("Comando cd \n");
    char directorio []= "/home/jamengual1/Escritorio/FlashDRIVE";
    printf("%s", args+1);
    if (chdir(directorio) == -1){
        fprintf(stderr, "Error %d: %s\n", errno, strerror(errno));
        perror("Error");
        return -1;
    } else{
        printf("Estás en el directorio: %s \n", getcwd(buff, 50));
        return 1;
    }

}

   //así es como tendría que ser pasándole args pero me da Bad Address todo el rato
/*int internal_cd(char **args){
    printf("%s","cambio de directorio\n");
    char buff[50];
    printf("Comando cd \n");
    //printf("%s", args[1]); //violación de segmento
    if (chdir(args[1]) == -1)
    {
        //fprintf(stderr, "Error %d: %s\n", errno,strerror(errno));
        perror("Error");
        return -1;
    }

    printf("Estás en el directorio: %s \n", getcwd(buff, 50));
    return 1;

}
*/

int internal_export(char **args) {
    printf("%s","éste es el export\n");
    char *variable;
    char *nuevo_valor;
    char *aux;

    variable = strtok(args[1], "=");
    nuevo_valor = strtok(NULL, args[1]);
    aux = getenv(variable);

    if((int)aux == -1)
    {
        perror("Error: getenv");
        return -1;
    }
        printf("VAR: '%s'. Valor: '%s'. Nuevo valor: '%s'\n", variable, aux, nuevo_valor);

    if(!nuevo_valor){
        perror("Error: error de sintaxis");
        return -1;

    }

        if ((setenv(variable,nuevo_valor,1)== -1))
        {

            perror("Error: llamada al sistema con setenv");
            return -1;

        }

        aux = getenv(variable);

        if((int)aux == -1)
        {
             perror("Error: llamada al sistema con getenv");
            return -1;

        }

        printf("Nuevo valor: '%s': '%s'\n", variable, aux);
        return 1;        

}



int internal_source(char **args) {
    printf("%s","éste es el source\n");
    return 1;
}

int internal_jobs(char **args){
    printf("%s","éste es el jobs\n");
    return 1;
}

void main(){
    char line[MAX_LINE]; 
    while(read_line(line)){
        execute_line(line);
    }
}
    
asked by Januan 28.11.2016 в 00:10
source

2 answers

3
char *read_line(char *line){
    printf("%s%s ",getenv("USER"),PROMPT);
    fflush(stdout);
    line=fgets(line,MAX_LINE,stdin);
    return line;
}

Being strict, that function should look like this:

char *read_line(char *line){
    printf("%s%s ",getenv("USER"),PROMPT);
    fflush(stdout);
    return fgets(line,MAX_LINE,stdin);
}

Otherwise the result is a bit misleading, because when you reach the end of the file,'line' will point to 0 but that pointer change will not be reflected outside the function:

void func(int* ptr)
{
  ptr = 0;
}

int main()
{
  int* ptr = 1000;
  func(ptr);

  // Pregunta: ¿Cuanto vale ptr aqui?
  // Respuesta: 100
}

And now we focus on the error:

int execute_line(char *line){
    char **args; // (1)
    parse_args(args,line);
    check_internal(args);
    return 0;
}

int parse_args(char **args, char *line){
    int n=0;
    char* token;
    char delimit[]=" \t\r\n\v\f";
    token=strtok(line,delimit);
    while(token!=NULL){
        printf("token%i: %s\n",n,token);
        args=token; // (2) (3)
        n++;
        args++;
        token=strtok(NULL,delimit);
    }
    printf("token%i: %s\n",n,token);
    args=token; // (4)
    return n;
}
  • (1): args is a double pointer and is not initialized. Where does it point? Anywhere except for a properly initialized memory position
  • (2) you assign a value to args , which is not initialized then you are overwriting memory that does not belong to you.
  • (3) args is char** and token is char* . They are different types and the result of that assignment can not be good.
  • (4) What do you intend with this last assignment?

If you want to store a list in args you have several options:

  • Previously calculate the number of items to be stored and make the corresponding memory reserve. In this case together with args you will have to provide a variable that says what is the number of elements it contains.
  • You assume a maximum number and make args have a fixed size. In this case it would also be advisable to indicate the effective number of values that it stores.
  • An example with dynamic memory:

    int execute_line(char *line){
        char **args;
        int nargs;
        parse_args(&args,&nargs,line); // parse_args requiere un puntero triple
        check_internal(args);
        free(args);
        return 0;
    }
    
    int parse_args(char ***args, int* nargs char *line){
        int n=0;
        const char delimit[]=" \t\r\n\v\f";
    
        *nargs = 0;
        char cadTemp[MAX_LINE];
        strcpy(cadTemp,line);
        char* token=strtok(cadTemp,delimit)
        while( token )
        {
          *nargs++;
          token=strtok(NULL,delimit);
        }
    
        *args = malloc(*nargs * sizeof(char*);
        token=strtok(linea,delimit)
        while(token!=NULL){
            printf("token%i: %s\n",n,token);
            **args=token;
            n++;
            (*args)++;
            token=strtok(NULL,delimit);
        }
        printf("token%i: %s\n",n,token);
        return n;
    }
    

    Why do you need a triple pointer? Because otherwise the changes will be local (This has already been explained in other questions, such as this

        
    answered by 28.11.2016 в 00:45
    2

    Your code contained a lot of errors.

    This is my version, keeping the interface of your functions intact.

    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include <errno.h>
    #include <unistd.h>
    
    #define PROMPT "$"
    #define MAX_LINE 512
    
    #define DELIMS " \t\r\n\v\f"
    
    int parse_args( char **args, char *line){
      int n = 0; // Contador de palabras.
      char *token = strtok( line, DELIMS );
    
      while( token ) {
        args[n] = token;
        ++n;
        token = strtok( NULL, DELIMS );
      }
      return n;
    }
    
    char *read_line( char *line ){
      printf( "%s%s ", getenv( "USER" ), PROMPT );
      fflush( stdout );
      return fgets( line, MAX_LINE, stdin );
    }
    
    int execute_line( char *line ){
      char *args[MAX_LINE/2]; // NO puede haber mas de MAX_LINE / 2 ordenes.
    
      memset( args, 0, MAX_LINE ); // Por seguridad.
      parse_args( args, line );
    
      return check_internal( args );
    }
    
    int check_internal( char **args ) {
      if( !strcmp( args[0], "cd" ) ) {
        return internal_cd( args );
      } else
      if( !strcmp(args[0], "export" ) ) {
        return internal_export( args );
      } else
      if( !strcmp( args[0], "source" ) ) {
        return internal_source();
      } else
      if( !strcmp(args[0], "jobs" ) ) {
        return internal_jobs( );
      } else
      return -1;
    }
    
    int internal_cd( char **args ){
      char buff[MAX_LINE];
      int ret;
    
      if( !args[1] ) {
        printf( "Debe indicar una ruta.\n" );
      } else {
        if( chdir( args[1] ) == -1 ) {
          fprintf( stderr, "Error: %s\n", strerror( errno ) );
          return -1;
        }
        getcwd( buff, MAX_LINE );
        printf( "Estas en la ruta %s\n", buff );
      }
      return 1;
    }
    
    int internal_export( char **args ) {
      char buff[MAX_LINE];
      char *old;
      int ret;
    
      if( args[1] && args[2] ) {
        old = getenv( args[1] );
        printf( "Valor antiguo: %s\n", old ? old : "SIN ASIGNAR" );
        if( setenv( args[1], args[2], 1 ) == -1 ) {
          fprintf( stderr, "Error: %s\n", strerror( errno ) );
          return -1;
        }
        printf( "Valor nuevo: %s\n", args[2] );
      } else {
        fprintf( stderr, "Error. Uso: export <nombre> <valor>\n" );
      }
      return 1;
    }
    
    int internal_source(char **args) {
      printf("%s","éste es el source\n");
      return 1;
    }
    
    int internal_jobs(char **args){
        printf("%s","éste es el jobs\n");
        return 1;
    }
    
    void main( ) {
      char line[MAX_LINE]; 
    
      while( read_line( line ) ) {
        execute_line( line );
      }
    }
    

    I do not detail all the modifications because, honestly, they have been enough and I do not remember them.
    As the interface of your functions is maintained, you can compare them.

    EDITO

    The big trick is in the execute_line( ) and parse_args( ) functions:

    • In execute_line( ) , we created an array of pointers to save precisely that, pointers to the tokens that the user passed us.
      Why [MAX_LINE / 2] ? Very easy. The worst possible case is that the user will pass something like "a b c d e f g h" . In that case, for every 2 characters, we would have a token ; hence the / 2 . Any other entry will contain fewer tokes.

    • In parse_args( ) we receive the array that we created in the previous function. strtok( ) modifies its first argument, inserting 0 where it finds delimiters, and returns pointers to the tokens created. We just keep those pointers. Nothing else is needed, strtok( ) takes care of everything, and we reuse the original line entered by the user in the rest of the program.

    How many tokes did the user enter? We do not know; but if we know that args[] was filled with 0 , so the first token not valid is a null pointer. We just have to check it out.

    Pd.
    That NULL == 0 == <FALSE> is the best invention in the world world!

        
    answered by 20.12.2016 в 10:14