At the beginning of my program I created this struct:
struct directive {
char gate[17];
int n;
int s;
int *inputs;
int *outputs;
int *selectors;
};
Included here is my whole main function:
int main(int argc, char** argv) {
if (argc - 1 != 1) {
printf("Invalid number of arguments\n");
return 0;
}
//get file, return if invalid path
FILE *file = fopen(argv[1], "r");
if (!file) {
printf("Invalid input\n");
return 0;
}
//make temp of circuit with struct directive
int scount = 0;
struct directive* temp = NULL;
int size = 2;
int icount = 0;
int ocount = 0;
int tcount = 0;
char dir[17];
char **names;
int *values;
//get INPUT info
fscanf(file, " %s", dir);
fscanf(file, "%d", &icount);
size = icount;
names = malloc(size * sizeof(char *));
names[0] = malloc(2 * sizeof(char)); //MALLOC
strcpy(names[0], "0");
names[1] = malloc(2 * sizeof(char)); //MALLOC
strcpy(names[1], "1");
int i;
for (i = 0; i < icount; i ) {
names[i 2] = malloc(17 * sizeof(char));
fscanf(file, "%*[: ]s", names[i 2]);
}
//get OUTPUT info
fscanf(file, " %s", dir);
fscanf(file, "%d", &ocount);
size = ocount;
names = realloc(names, size * sizeof(char *));
for (i = 0; i < ocount; i ) {
names[i icount 2] = malloc(17 * sizeof(char));
fscanf(file, "%*[: ]s", names[i icount 2]);
}
//get temp
struct directive step;
while (!feof(file)) {
int numInputs = 2, numOutputs = 1;
int sc = fscanf(file, " %s", dir);
if (sc != 1) {
break;
}
scount ;
step.n = 0;
step.s = 0;
strcpy(step.gate, dir);
if (strcmp(dir, "NOT") == 0) {
numInputs = 1;
}
if (strcmp(dir, "PASS") == 0) {
numInputs = 1;
}
if (strcmp(dir, "DECODER") == 0) {
fscanf(file, "%d", &numInputs);
step.n = numInputs;
numOutputs = pow(2, numInputs);
}
if (strcmp(dir, "MULTIPLEXER") == 0) {
fscanf(file, "%d", &numInputs);
step.s = numInputs;
numInputs = pow(2, numInputs);
}
step.inputs = malloc(numInputs * sizeof(int));
step.outputs = malloc(numOutputs * sizeof(int));
step.selectors = malloc(step.s * sizeof(int));
char v[17];
for (i = 0; i < numInputs; i ) {
fscanf(file, "%*[: ]s", v);
step.inputs[i] = indexOf(size, names, v);
}
for (i = 0; i < step.s; i ) {
fscanf(file, "%*[: ]s", v);
step.selectors[i] = indexOf(size, names, v);
}
for (i = 0; i < numOutputs; i ) {
fscanf(file, "%*[: ]s", v);
int idx = indexOf(size, names, v);
if (idx == -1) {
size ;
tcount ;
names = realloc(names, size * sizeof(char *));
names[size - 1] = malloc(17 * sizeof(char));
strcpy(names[size - 1], v);
step.outputs[i] = size - 1;
}
else {
step.outputs[i] = idx;
}
}
//add step to list of temp
if (!temp) {
temp = malloc(sizeof(struct directive));
} else {
temp = realloc(temp, scount * sizeof(struct directive));
}
temp[scount - 1] = step;
}
// initialize values array
values = malloc(size * sizeof(int));
resetValues(size, values);
while(1 < 2) {
//print inputs
for (i = 0; i < icount; i ) {
printf("%d ", values[i 2]);
}
printf("|");
//run through temp, calculate outputs
for (i = 0; i < scount; i ) {
struct directive step = temp[i];
if (strcmp(step.gate, "NOT") == 0) {
NOT(values, step.inputs[0], step.outputs[0]);
}
if (strcmp(step.gate, "AND") == 0) {
AND(values, step.inputs[0], step.inputs[1], step.outputs[0]);
}
if (strcmp(step.gate, "OR") == 0) {
OR(values, step.inputs[0], step.inputs[1], step.outputs[0]);
}
if (strcmp(step.gate, "NAND") == 0) {
NAND(values, step.inputs[0], step.inputs[1], step.outputs[0]);
}
if (strcmp(step.gate, "NOR") == 0) {
NOR(values, step.inputs[0], step.inputs[1], step.outputs[0]);
}
if (strcmp(step.gate, "XOR") == 0) {
XOR(values, step.inputs[0], step.inputs[1], step.outputs[0]);
}
if (strcmp(step.gate, "PASS") == 0) {
PASS(values, step.inputs[0], step.outputs[0]);
}
if (strcmp(step.gate, "DECODER") == 0) {
DECODER(values, step.n, step.inputs, step.outputs);
}
if (strcmp(step.gate, "MULTIPLEXER") == 0) {
MUX(values, step.s, step.inputs, step.selectors, step.outputs[0]);
}
}
//print outputs
for (i = 0; i < ocount; i ) {
printf(" %d", values[icount i 2]);
}
printf("\n");
if (!incrementInputs(values, icount)) {
break;
}
}
for (i = 0; i < icount; i ) {
free(names[i 2]);
}
for (i = 0; i < ocount; i ) {
free(names[i icount 2]);
}
free(step.inputs);
free(step.outputs);
free(step.selectors);
free(names[0]);
free(names[1]);
free(values);
free(temp);
return 0;
}
The problem that I am having is when compiled, my address sanitizer is telling me that I have a memory leak at lines 203 and 204 which are these following mallocs:
names = realloc(names, size * sizeof(char *));
names[size - 1] = malloc(17 * sizeof(char));
You can see that I freed the three lines that I had allocated for, so what could be the reason that it isn't actually freeing? Do I have to loop through something and use free because of something else I did after in main?
CodePudding user response：
You allocate memory in a loop but only free what you obtained in the last iteration:
...
while (!feof(file)) {
...
step.inputs = malloc(numInputs * sizeof(int));
step.outputs = malloc(numOutputs * sizeof(int));
step.selectors = malloc(step.s * sizeof(int));
...
}
...
free(step.inputs);
free(step.outputs);
free(step.selectors);
...
CodePudding user response：
This group of calls is only freeing the last held values of step, as a result of the final iteration of the while (!feof(file)) { ... } loop.
free(step.inputs);
free(step.outputs);
free(step.selectors);
Every iteration, step is copied to an element of temp, so you must free the members of each element before freeing temp.
for (int i = 0; i < scount; i ) {
free(temp[i].inputs);
free(temp[i].outputs);
free(temp[i].selectors);
}
CodePudding user response：
Others have already pointed out the main issues, so I won't repeat them, but I want to give a few hints to make allocation a bit safer.
When you malloc() and need to provide the size of data, there is nothing wrong with using sizeof(type), but if you get the type wrong, just as a simple typo, you get the wrong amount of memory. Even if you don't make any mistakes, it can bite you if you change a pointer type at some later point. The compiler doesn't check if the size you give malloc() is correct (it can't, really), so you don't get any warning about places where you allocated size for the wrong type.
So, in allocations such as
names = malloc(size * sizeof(char *));
names[0] = malloc(2 * sizeof(char));
names[i 2] = malloc(17 * sizeof(char));
step.inputs = malloc(numInputs * sizeof(int));
step.outputs = malloc(numOutputs * sizeof(int));
step.selectors = malloc(step.s * sizeof(int));
all the sizeof() are potential hiding places for bugs.
You can, instead, get the size from the pointer you are assigning the allocation to. Not always, of course, there are times where you need to allocate something and you don't have a variable you assign to--sending allocated memory to a function or some-such, but 99% of the time, if you malloc you are assigning to a variable. And you can get the size of the type that variable points to using sizeof as well. The sizeof operator comes in two flavours. One that takes a type, sizeof(type) and one that takes an expression, sizeof expressions or sizeof(expression); the parentheses are optional here.
You can change the allocations above to this:
names = malloc(size * sizeof *names);
names[0] = malloc(2 * sizeof *names[0])
names[i 2] = malloc(17 * sizeof *names[i 2])
step.inputs = malloc(numInputs * sizeof *step.inputs);
step.outputs = malloc(numOutputs * sizeof *step.outputs);
step.selectors = malloc(step.s * sizeof *step.selectors);
You need to dereference the variable here, because otherwise you get the size of the pointer and not what it points to, but don't worry about this dereference. The compiler is not going to emit code that actually dereferences, which in any case would be disastrous considering that the pointers are not initialised yet. No, the pointer just works out the type of the expression and gets the size from there. That way, the allocation and the type of what you want to allocate space for is kept consistent.
Another thing is the realloc() you do:
names = realloc(names, size * sizeof(char *));
It is, admittedly, unlikely to fail, but if it does, realloc() returns NULL, and if it does, it hasn't freed its input pointer. If this realloc() failed, you would have NULL in names, and you would have leaked the memory that names pointed at before.
You generally shouldn't assign the result from realloc() directly to the input pointer for that reason. It is better to assign to a temporary variable, check if the allocation failed, and if not, then you can assign to the variable you want to hold the result.
char **tmp = realloc(names, size * sizeof *tmp);
if (!tmp) abort(); // handle alloc failure
names = tmp;
This, of course, depends on whether you plan to handle allocation failures. I just abort() here, and you could do the same without assigning to a tmp variable
names = realloc(names, size * sizeof *tmp);
if (!names) abort(); // handle alloc failure
If you are running on a desktop, allocation errors are unlikely to ever be a problem, and terminating the program with some appropriate error message is an easy fix that is probably fine.