I am trying to parse an input for my simple shell program to execute, though it is working as expected but i am having a **valgrind memory leak and invalid free() error **. I have tried to figure out the issue but to no avail, Thanks in advance
#cisfun$ /bin/ls -l
token: /bin/ls
arg_list[0]: /bin/ls
token: -l
arg_list[1]: -l
token: (null)
cmd: /bin/ls
total 204
-rwxrw-r-- 1 vagrant vagrant 104 Jul 2 12:39 AUTHORS
-rwxrw-r-- 1 vagrant vagrant 23 Jun 14 16:25 README.md
-rw-rw-r-- 1 vagrant vagrant 575 Jul 4 08:25 execute.c
-rw-rw-r-- 1 vagrant vagrant 197 Aug 13 09:17 free_arg.c
-rwxr-xr-x 1 vagrant vagrant 142144 Jul 3 05:04 hbtn_ls
-rw-rw-r-- 1 vagrant vagrant 810 Aug 13 09:15 parse_arg.c
-rwxrw-r-- 1 vagrant vagrant 789 Jul 4 12:05 puts.c
-rwxrwxr-x 1 vagrant vagrant 27888 Aug 13 09:15 shell
-rwxrw-r-- 1 vagrant vagrant 1186 Aug 13 10:47 shell.c
-rwxrw-r-- 1 vagrant vagrant 398 Jul 4 10:29 shell.h
-rw-rw-r-- 1 vagrant vagrant 156 Jul 4 08:31 sig_handler.c
#cisfun$ ^Z
==8999== Invalid free() / delete / delete[] / realloc()
==8999== at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==8999== by 0x109423: free_arg (free_arg.c:12)
==8999== by 0x10991C: main (shell.c:77)
==8999== Address 0x4a4f040 is 0 bytes inside a block of size 120 free'd
==8999== at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==8999== by 0x10990D: main (shell.c:76)
==8999== Block was alloc'd at
==8999== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==8999== by 0x48DD573: getdelim (iogetdelim.c:62)
==8999== by 0x1097EB: main (shell.c:47)
==8999==
==8999==
==8999== HEAP SUMMARY:
==8999== in use at exit: 11 bytes in 2 blocks
==8999== total heap usage: 8 allocs, 8 frees, 2,227 bytes allocated
==8999==
==8999== 11 bytes in 2 blocks are definitely lost in loss record 1 of 1
==8999== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==8999== by 0x109528: parse_arg (parse_arg.c:29)
==8999== by 0x109843: main (shell.c:54)
==8999==
==8999== LEAK SUMMARY:
==8999== definitely lost: 11 bytes in 2 blocks
==8999== indirectly lost: 0 bytes in 0 blocks
==8999== possibly lost: 0 bytes in 0 blocks
==8999== still reachable: 0 bytes in 0 blocks
==8999== suppressed: 0 bytes in 0 blocks
==8999==
==8999== For lists of detected and suppressed errors, rerun with: -s
==8999== ERROR SUMMARY: 3 errors from 2 contexts (suppressed: 0 from 0)
This is my main function
47 nread = getline(&line, &len, stdin);
48 if (nread == -1)
49 break;
50
51 if (line[nread - 1] == '\n')
52 line[nread - 1] = '\0';
53
54 arg_list = parse_arg(line);
55
56 cmd = arg_list[0];
57 printf("cmd: %s\n", cmd);
58 if (cmd == NULL || *cmd == '\0')
59 continue;
60
61
62
63 if (stat(cmd, &st) == 0)
64 {
65 execute(arg_list, prog_name);
66 }
67 else
68 {
69 perror(prog_name);
70 }
71 }
72
73 if (isatty(STDIN_FILENO))
74 _puts("\n");
75
76 free(line);
77 free_arg(arg_list);
78 return (0);
and the Parse_arg() and free_arg() function;
7 char **parse_arg(char *arg)
8 {
9 char **arg_list;
10 char *str, *token;
11 int i, j = 1;
12
13 arg_list = malloc(sizeof(*arg_list) * j);
14 if (arg_list == NULL)
15 return (NULL);
16
17 for (i = 0, str = arg; ;str = NULL, i )
18 {
19 token = strtok(str, " ");
20 printf("token: %s\n", token);
21
22 if (token == NULL)
23 {
24 arg_list[i] = NULL;
25 break;
26 }
27 else
28 {
29 arg_list[i] = malloc(sizeof(char) * strlen(token) 1);
30 if (arg_list[i] == NULL)
31 {
32 while (i--)
33 free(arg_list[i]);
34
35 free(arg_list);
36 return (NULL);
37 }
38 arg_list[i] = token;
39 printf("arg_list[%d]: %s\n", i, arg_list[i]);
40 }
41 arg_list = realloc(arg_list, sizeof(*arg_list) * j);
42 if (arg_list == NULL)
43 return (NULL);
44
45 }
46 return (arg_list);
47 }
6 void free_arg(char **arg)
7 {
8 int i;
9
10 for (i = 0; arg[i] != NULL; i )
11 {
12 free(arg[i]);
13 }
14 free(arg);
15 }
CodePudding user response:
arg_list
contains a pointer to line
. So line
is being freed twice. Once in main (76) and once in free_arg. Remove the line free(line)
in your main function.
CodePudding user response:
29 arg_list[i] = malloc(sizeof(char) * strlen(token) 1);
30 if (arg_list[i] == NULL)
31 {
32 while (i--)
33 free(arg_list[i]);
34
35 free(arg_list);
36 return (NULL);
37 }
// arg_list[i]` points at an allocated buffer.
38 arg_list[i] = token;
// But, ignore that buffer and reassign it to a local variable
// The allocated buffer is now orphaned
You probably want `strcpy( arg_list[i], token );