Homework: I have a memory leak somewhere, but I can’t find it. Any tips on how to use valgrind more efficiently?

Question

Well, first an honest warning, this is a project for the class. I am not looking for any help other than fixing a memory leak. I guess I followed some terrifying coding methods in the space of this C code. Despite this, when I run Valgrind to look for a memory leak, it is completely unclear to me what memory leaks are missing. I know that there are at least two lines that I do not free, but I malloc () ed based solely on the output size of valgrind. Since I took some of the extraneous code from the project, valgrind line numbers may well be turned off, because of this I marked them with comments for your convenience.

A small background, I write the basic functions of the shell. He currently performs the following actions:

1. Enter user input

2. Enter the cmdin structure

3. Run the command if it does not have a channel.

4. free up the space created by cmdin, and start again in step 1. Here the problem arises.

#include <stdio.h> #include <string.h> #include <stdlib.h> #include <unistd.h> #include <sys/wait.h> #define MAX_SIZE 1024 #define MAX_CLEN 2097152 #define READ_END 0 #define WRITE_END 1 #define CHILD_STATUS 0 struct cmdin { char *cmd; char **args; int nargs; int pipeflag; }; //check if the last argument of the command passed to it is a pipe. //if so, return the position of the pipe, otherwise, return 0 //if it has already found a pipe, it will return the position of that pipe int conpipe(struct cmdin * cmd) { if(!cmd->pipeflag) { if(!strcmp(cmd->args[cmd->nargs], "|")) { return cmd->nargs; } else { return 0; /* PROBLEM LINE BELOW */ } // line 46, where valgrind claims one of the problems exists } else { //printf("pipeflag: %d\n", cmd->pipeflag); return (cmd->pipeflag); } } //free the command after each runthrough int freeze(struct cmdin cmd) { int i; for(i=0; i <= (cmd.nargs); i++) { //printf("cmd.args[%d]: %s\n",i, cmd.args[i]); /* PROBLEM LINE BELOW*/ free(cmd.args[i]); //this is line 62, as noted by valgrind } free(cmd.args); free(cmd.cmd); return 0; } //parse input, and add a null to the end struct cmdin * parse(char *cmd) { //allocate space for the command struct cmdin *ped = malloc(sizeof(struct cmdin)); //declare pipeflag, and nargs as 0 ped->pipeflag = 0; ped->nargs = 0; //allocate space for the array of strings, and for the string cmd. ped->args = malloc(sizeof(char*) * MAX_SIZE); ped->cmd = malloc(sizeof(char) * MAX_SIZE); //scan the input, and put the first argument into the cmd string sscanf(cmd, "%s %[^\n]", ped->cmd, cmd); //allocate space for the next string, and then copy cmd to it. ped->args[ped->nargs] = malloc(sizeof(char) * MAX_SIZE); strcpy(ped->args[ped->nargs], ped->cmd); ped->pipeflag = conpipe(ped); /* PROBLEM LINE BELOW*/ ped->nargs++; // line 86, where valgrind claims the second leak is called? ped->args[ped->nargs] = malloc(sizeof(char) * MAX_SIZE); //loop that allocates space for a command, and then assigns //the next arg to it. while(sscanf(cmd, " %s %[^\n]", ped->args[ped->nargs], cmd) == 2) { ped->pipeflag = conpipe(ped); ped->nargs++; ped->args[ped->nargs] = malloc(sizeof(char) * MAX_SIZE); } strncpy(ped->args[ped->nargs], cmd, strlen(cmd)-1); ped->nargs++; //ped->args[ped->nargs] = malloc(sizeof(char) * MAX_SIZE); ped->args[ped->nargs] = NULL; return ped; } int main() { char cwd[MAX_CLEN]; getcwd(cwd, sizeof(cwd)); char input[MAX_CLEN]; char *exit = "exit\n"; char *cd = "cd"; //Command to take input printf("tosh$ "); fgets(input, sizeof input, stdin); while(strcmp(input, exit)) { //Command to parse input struct cmdin *cmd; cmd = parse(input); //if there is not a pipeflag if(!cmd->pipeflag) { //Change directories if(!strcmp(cd, cmd->args[0])) { chdir(cmd->args[1]); getcwd(cwd, sizeof(cwd)); } else if(strcmp(input, exit)) { //command to run input int child_pid; child_pid = fork(); if(child_pid == 0) { if(strcmp(cmd->args[1],cmd->args[0])) { execvp(cmd->cmd, cmd->args); } else { free(cmd->args[1]); cmd->args[1] = NULL; cmd->nargs--; execvp(cmd->cmd, cmd->args); } } else { wait(&child_pid); } } freeze(*cmd); free(cmd); } //Command to take input printf("tosh$ "); fgets(input, sizeof input, stdin); } return 0; } 

Note: the input for this valgrind is as follows:

 tosh$ ls -al tosh$ exit 

Heap and leak briefs:

 HEAP SUMMARY: ==4901== in use at exit: 4,096 bytes in 2 blocks ==4901== total heap usage: 6 allocs, 4 frees, 24,600 bytes allocated ==4901== ==4901== 2,048 bytes in 1 blocks are definitely lost in loss record 1 of 2 ==4901== at 0x4C2B3F8: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==4901== by 0x400A48: parse (tosh.c:46) ==4901== by 0x400C97: main (tosh.c:86) ==4901== ==4901== 2,048 bytes in 1 blocks are definitely lost in loss record 2 of 2 ==4901== at 0x4C2B3F8: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==4901== by 0x400BDE: parse (tosh.c:62) ==4901== by 0x400C97: main (tosh.c:86) ==4901== ==4901== LEAK SUMMARY: ==4901== definitely lost: 4,096 bytes in 2 blocks ==4901== indirectly lost: 0 bytes in 0 blocks ==4901== possibly lost: 0 bytes in 0 blocks ==4901== still reachable: 0 bytes in 0 blocks ==4901== suppressed: 0 bytes in 0 blocks 

UPDATE: as requested. My Makefile:

 CC=gcc CFLAGS=-g -Wall TARGET=tosh $(TARGET): $(TARGET).c $(CC) $(CFLAGS) -o $(TARGET) $(TARGET).c 

Valgrind Version: Valgrind-3.7.0

gcc Version: gcc (Ubuntu / Linaro 4.7.2-2ubuntu1) 4.7.2

echo $ LD_PRELOAD:

(It didn't print anything)

Answer 1

Failed to play. But a few notes.

• Use what is available for errors and warnings.

   gcc -Wall -Wextra -pedantic … 

• Check return values ​​and operation success. This is necessary to reduce errors, especially those that show only the case of a user who is a mile from the road.

 if (!(foo = malloc(size))) { perror("mem"); } ... /* This line is no good: */ sscanf(cmd, "%s %[^\n]", ped->cmd, cmd); /* | | * +----------+----------------+ * | * +--- Not good. UB. (Input / Output same.) */ 

You can add a routine for counting and sizes ...

 i = sscanf(cmd, "%s%n %[^\n]%n", ped->cmd, &n1, cmd, &n2); if (i < 2) { /* Debug print. */ fprintf(stderr, "%s:%-3d; " "sscanf => %d items and %d bytes str[%d]<%s>\n", __FILE__, __LINENO_, i, n2, n1, ped->cmd ); } 

and etc.

Answer 2

Ok, I just realized what the problem is. Most commentators were right. It was that I was working with valgrind on a different source code. I save a copy of the tosh in my ~ / usr / bin (which is in my $ PATH). I forgot to update this piece of code with my latest changes (including those that fixed memory leak problems).

When I ran valgrind, I used the following command from the directory containing tosh.c tosh:

$ valgrind tosh

I had to run:

$ valgrind ./tosh

When I made an update for my team, the code worked fine, without memory leaks. Thank you for your help! Sorry, it took me so long to figure this out.