0

I need to make a program that calls a new program with execlp and then send its output back to the main program that will output and modify it to the standard output. As for the program it works just fine but when I test it with valgrind it gives me :

Invalid read of size 1 
Address 0x0 is not stack'd, malloc'd or (recently) free'd

the error line is: strcpy(program,argv[optind]); I don`t know why is it an issue if I copy argv[optind] into program that was malloced.

#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>
#include <wait.h>
#include <ctype.h>
#include <string.h>

int main(int argc, char** argv){

    int c,U=0,l=0,x=0,r=0;
    extern char *optarg;
    extern int optind;
    extern int optopt;
    while ( (c = getopt(argc, argv, ":Ulxr")) != -1) {
        switch (c) {
            case 'U': U = 1;
                      break;
            case 'l': l = 1;
                      break;
            case 'x': x = 1;
                      break;
            case 'r': r = 1;
                      break;
        }
    }
    char* program = (char*)malloc(sizeof(argv[optind]));
    strcpy(program,argv[optind]);
    int nov_stdin[2];
    int nov_stdout[2];
    if(pipe(nov_stdin)!=0){
        perror("tezava pri ustvarjanju cevi za stdin");
        return -1;
    }
    if(pipe(nov_stdout)!=0){
        perror("tezava pri ustvarjanju cevi za stdout");
        return -1;
    }
    int child_pid; // tukaj bomo hranili pid otroka, ki ga vrne fork
    switch(child_pid=fork()){
        case -1:
            perror("tezava pri klicu fork");
            return 0;

        case 0:
            close(nov_stdout[0]); // stdout zelimo pisati, zato zapremo branje
            dup2(nov_stdout[1], 1); // zapremo 1 in ga zamenjamo z nov_stdin[1]
            execlp(program, program, (char*)0);
            return 0;
    }
    free(program);
    close(nov_stdin[0]);
    close(nov_stdout[1]);
    char data;
    while(read(nov_stdout[0], &data, 1) > 0){
        if(l == 1){data = tolower(data);}
        if(U == 1){data = toupper(data);}
        if(r == 1){
            int new_data = (char)data;
            if(new_data == 10){
                data = ' ';
            }
        }
        if(x != 1){
        printf("%c", data);
        }else{
            if(isprint(data)){
                printf("%c", data);
            }else{
                printf("10");
            }
        }
    }
    printf("\n");
    wait(0);
    return 0;
}

Thank you for your help.

PS: I also noticed that the test program on my schools page is saying this:

    make clean all

rm -f main main.o
gcc -Wall -std=c99   -c -o main.o main.c
main.c: In function ‘main’:
main.c:19:5: warning: implicit declaration of function ‘getopt’ [-Wimplicit-function-declaration]
     while ( (c = getopt(argc, argv, ":Ulxr")) != -1) {
     ^
main.c:18:16: warning: unused variable ‘optopt’ [-Wunused-variable]
     extern int optopt;
                ^
main.c:16:18: warning: unused variable ‘optarg’ [-Wunused-variable]
     extern char *optarg;
                  ^
gcc -Wall -std=c99 main.o -o main

But the program works and compiles just fine on my pc. Fixed this issue with adding #include <getopt.h>

getopt issue

PS: FIXED, I was not testing if any arguments were given.

Aleš Kovačič
  • 1
  • 1

1 Answers1

0

sizeof does not do what you think it does. Change:

char* program = (char*)malloc(sizeof(argv[optind]));

to:

char* program = malloc(strlen(argv[optind]) + 1);

Notes:

Paul R
  • 208,748
  • 37
  • 389
  • 560
  • with char* program = malloc(strlen(argv[optind]) + 1) valgrind found an issue in strlen. ==14085== Invalid read of size 1 ==14085== at 0x4C2E0E2: strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) – Aleš Kovačič May 27 '17 at 09:36
  • I'm guessing that you are not supplying any command line arguments, so `argv[optind]` is probably invalid. Your code should do a sanity check for the required arguments and exit gracefully if they are not present. – Paul R May 27 '17 at 10:45
  • Yes! that would be the case, I was only testing on my PC with arguments but the school does a blank test. Thank you – Aleš Kovačič May 27 '17 at 10:47