Code keeps returning the error "control reaches end of non-void function".
The source of the error is probably: int lps[M]
but I can't figure out how to fix it. I tried to assign value to it but the error is still there.
EDIT: sorry about the return -1; thing. Fixed that but the error I'm trying to fix is still there.
int knuth(char *string, char * pattern){
int M = strlen(string);
int N = strlen(pattern);
int lps[M];
if (string == NULL)
{
return -1;
}
else {
int i = 0;
int j = 0;
while (i < N) {
if (string[j] == pattern[i]) {
j ;
i ;
}
if (j == M) {
//printf("Found stringtern at index %d ", i - j);
j = lps[j - 1];
return i - j;
} else if (i < N && string[j] != pattern[i]) {
if (j != 0)
j = lps[j - 1];
else
i = i 1;
}
return i - j;
}
}
}
CodePudding user response:
Ignoring all other potential problems in this function, the compiler is rightfully complaining that not all code paths return
a value. You have declared the knuth
function to return an int
, yet it's possible for logic in your function to reach the end without return
ing a value. If you go down one of those paths, the function will return an indeterminate value, and if the caller uses that value, it will invoke undefined behavior. See the condensed version of your function below:
int knuth(char *string, char * pattern){
if (string == NULL)
{
return -1; // ok, here's a return, but it's conditional on string == NULL
}
else {
while (...) {
if (...) {
return i - j; // here's another return, also conditional
} else if (...) {
}
return i - j; // here's the final return in your function, which is
// conditional on entering the while loop
}
// what if you get here (ie, the condition of the while loop was false)?
// The function returns nothing, this is what the compiler is warning
// you about.
}
// Here is logically the same as at the end of the else block .. also no return
}
As I mentioned in my comment, there's no point to the while
loop, since you unconditionally return
from it at the end of the first iteration. That means the loop is guaranteed to only execute once, eliminating the need for a loop all together. Your logic can be simplified, at least with a method as below:
int knuth(char *string, char * pattern){
int i=0, j=0;
if (string == NULL)
{
return -1; // ok, here's a return, but it's conditional on string == NULL
}
else if (i < N) {
// if i < N, do whatever manipulations you want as before
if (...) {
// return i - j; // really no need to return i-j here, we'll do that
// at the end now
} else if (...) {
}
}
// now at the very end of the function, this will unconditionally return
return i - j;
}
However, I suspect you actually want to loop, in which case your logic should be something like
int knuth(char *string, char * pattern){
int i=0, j=0;
if (string == NULL)
{
return -1;
}
else
{
while (i < N)
{
// ... do your operations. Be _very_ careful to increment
// i appropriately (probably each time thru the loop), otherwise
// you'll get an infinite loop here. I see a conditional
// mutation of i in your OP, which makes me nervous.
}
// since this is the end of an else block, we can return here
//return i - j;
}
// but IMO it's clearer to return here. It's up to you, depending on
// where you want to scope i and j
return i - j;
}
CodePudding user response:
Your code falls off the end of the function when string == NULL
.
This writing style is old; typically we would not write
if(string != NULL)
{
/* Entire body logic */
}
but rather we would now write
if(string == NULL)
{
/* handle input error and return; or do we do string = ""; */
}
You will find this mistake more easily avoided by using the modern style. It has been pointed out to me that string
can't be NULL
here because of the strlen(string)
call above. While such a platform does exist where strlen()
checks for NULL
, depending on that behavior is unwise, and the check should be moved before the strlen(string)
call.
Nobody believes in single return anymore, which was the only real reason for the old style.