Reputation: 85
The function in question:
struct node* findNext(struct node *root, struct node *ldr, int *p) {
// Check if there are nodes in the tree.
if(p == 0){
if (root != NULL) {
// correct organ/bt combo
if(cmpOrgan(root, ldr) == 1){
if (strcmp(root->organ.name, ldr->organ.name) != 0){
if(cmpDates(root, ldr) == 1){
p = 1;
return root;
}
// The leader has been in longer then root
if(cmpDates(root, ldr) == 2){
return findNext(root->left, ldr, p);
}
}
else{
findNext(root->left, ldr, p);
findNext(root->right, ldr, p);
}
}
if(cmpOrgan(root, ldr) == 2){
return findNext(root->left, ldr, p);
}
if(cmpOrgan(root, ldr) == 3){
return findNext(root->right, ldr, p);
}
}
}
return NULL;
}
I would like to break out of this recursive function at this part here:
if (strcmp(root->organ.name, ldr->organ.name) != 0){
if(cmpDates(root, ldr) == 1){
p = 1;
return root;
}
The way I'm trying to do this is by scanning in a global pointer to the function and changing it to 1 once I want the function to break. My goal is to return the current root at the break point. Is this possible to do? I'm initializing the pointer in main by doing this:
int *p = 0;
When I try to set the values with *p = 1, the program will crash. I i'm pretty sure I'm missing something fundamental with pointers but I just don't know what as I'm still new to coding. Can anyone help me? Is there another easier method then doing this that I'm not aware of? Thanks for any help in advance.
Upvotes: 1
Views: 2241
Reputation: 689
there is an error in the below portion of the code:
else{
findNext(root->left, ldr, p);
findNext(root->right, ldr, p);
}
you are losing the return value in both cases, which means any successful search in either path will be lost.
something like the below snippet should work:
else{
struct node *rv = findNext(root->left, ldr, p);
if (rv) {
return rv;
} else {
return findNext(root->right, ldr, p);
}
also no reason to call cmpOrgan three times, any level in the recursion each call to cmpOrgan should return the exact same value. same applies to cmpDates.
as every path returns a value. the variable p is no longer needed.
Upvotes: 0
Reputation: 4743
When you say int *p = 0
, you are creating a null pointer, as NULL == 0
.
When you then later attempt to say *p = 1
, you dereference the null pointer, causing your program to crash.
More correctly, if you really want to use a pointer, you should allocate memory for the int that you want to store there:
int *p = malloc(sizeof(int));
*p = 0;
and then you will have a place to put your int.
Alternately, don't bother creating it as a pointer -- just say
int p = 0;
and then pass the address of p
, using &p
, where you had passed p
before, changing your assignment to *p = 1
.
One third possibility would be to not pass p
at all, and just have a global variable p
that can get changed by the function at any level. This is generally considered bad coding style, but would accomplish what you seem to be trying to do.
Finally, note that you shouldn't really need to set a variable to break out of recursion -- when you return, it should simply continue returning all the way up your stack of function calls, which can generally be laid out so as to not need such a flag variable.
In particular, you make two recursive calls to findNext whose return values are simply discarded (inside your first else block). These don't seem to actually accomplish anything, and you'd most likely be better served keeping their return values and using those to decide what to return, or, better still, only calling one, checking its return value, and then making the second call only if necessary.
Upvotes: 3
Reputation: 1102
In findNext, you did the following
p = 1
But p is a pointer to an integer
, so you are actually setting the address of what p
points to equal to 1. This likely causes an access violation and a SIGSEGV.
This is why your program crashes. It tries to write data to an address outside of the address space of the application. Doing what qaphla says should work although I have not tested it myself.
Upvotes: 0