CODEWITHSUNDEEP
cmemorysegmentation-faultmalloc
lane0335
lane0335

Reputation: 125

Pointer Requires More Memory Allocation Than It Theoretically Should

Pretty new to C, but I thought I had the hang of allocating and managing memory until I ran into this issue recently.

I am working on a "make" utility. (It's not homework, just my friend's old assignment that I thought I could glean valuable practice from.) As I'm sure most of you know, makefiles have various targets, and these targets have depdendencies that must be attended to before the targets's commands can be executed.

In order to store data for a given target's dependencies found while parsing the makefile I made the following:

typedef struct{
    char* target;
    char** dependency_list;
}dependency_tracker;

In order to keep track of multiple dependency_trackers, I declared (and subsequently allocated for) the following variable. (NOTICE the "+4" after "total_number_of_targets". THE PROGRAM DOESN'T WORK WITHOUT IT, AND MY QUESTION IS WHY THAT IS.)

dependency_tracker** d_tracker_ptr = (dependency_tracker**) malloc((total_number_of_targets+4)*sizeof(dependency_tracker*));

I then sent the pointer for this to the parsing method with the following line:

parse_file(filename,&d_tracker_ptr);

Within the parse_file function, I believe these are the most important calls I make (left out string parsing calls). Note that target_counter is the number of targets parsed so far. I think everything else should be somewhat manageable to figure out:

dependency_tracker** tracker_ptr = *tracker_ptr_address; // tracker_ptr_address is the pointer I passed to the function above
// declare and allocate for the new struct we are creating
dependency_tracker* new_tracker_ptr = (dependency_tracker*) malloc(sizeof(dependency_tracker));
char* new_tracker_ptr_target = (char*) malloc((size_of_target)*sizeof(char)); // size_of_target is the string length
new_tracker_ptr->target = new_tracker_ptr_target;
*(tracker_ptr+target_counter*sizeof(dependency_tracker*)) = new_tracker_ptr;

As I mentioned earlier, I have to allocate space for four more (dependency_tracker*)'s than I would have thought I needed to in order for this program to complete without a segfault.

I came to the conclusion that this was because I was overwriting the space I had allocated for the pointer I pass to parse_file.

My question is: why does this happen? Even if space for a NULL pointer is needed, that shouldn't require the space of 4 additional pointers. And the program produces a segfault if I allocate anything less than 25 additional bytes in the original call to malloc

Let me know if anything needs clarification. I know this is a bit of a novel.

Upvotes: 0

Views: 70

Answers (2)

Gene
Gene

Reputation: 46960

This is broken:

*(tracker_ptr+target_counter*sizeof(dependency_tracker*)) = new_tracker_ptr;

The pointer size is accounted for by C. You want:

tracker_ptr[target_counter] = new_tracker_ptr;

Also as I mentioned in comments, you did not allow for a null terminator in the strings.

Another comment: C does not require a cast on malloc, and using one invites trouble. Also it's safer to just dereference the pointer you're assigning to inform sizeof. So just say:

dependency_tracker *new_tracker_ptr = malloc(sizeof *new_tracker_ptr);
char* new_tracker_ptr_target = malloc(size_of_target * sizeof *new_tracker_ptr_target);
dependency_tracker *new_tracker_ptr =  malloc(*new_tracker_ptr);
new_tracker_ptr->target = new_tracker_ptr_target;

Additionally, you may want to reconsider the vacuous words in your variable names. I'm actually a big fan of longish, explanatory identifiers, but "tracker" and "target" are so vague that they add little clarity. Similarly, embedding type information in variable names a la _ptr was a fad about 30 years ago. It's over now. If you have a function where the declaration and a variable name can't be grok'ed on the same screen, the function is too big.

Upvotes: 3

n. m. could be an AI
n. m. could be an AI

Reputation: 119867

*(tracker_ptr+target_counter*sizeof(dependency_tracker*)) = ...

This is the problem. Pointer arithmetic doesn't work like that. You do not have to multiply by sizeof(anyhing) when using properly typed (i.e. not char*) pointer arithmetic. What's better, you don't have to use pointer arithmetic at all. 

tracker_ptr[target_counter] = ...

is all that's needed.

Upvotes: 1

Related Questions