user2041427
user2041427

Reputation: 79

If statement being skipped

In this method I am looking to see if a string appears in word search. Since a string can appear in any direction I have methods like the following which check those directions:

Currently my code is giving a segmentation fault when run. This is happening because the first if statement in my method is being skipped according to gdb.

The grid I am working with looks like this:

a r e o 
o n l y
o d d a

The values of x, y and str at runtime (just before segfault) are:

x = 0;
y = 0;
str = "add"

Now strlen(str) evaluates to 3 and x - 3 evaluates to -3.

-3 < 0 would evaluate true and quits my method. Unfortunately the if statement at the start of the method is skipped at run time. Anyone know what is going on?

Furthermore I'll add that if I change the first line of my grid to:

q r e o

I get proper results.

Here is the method:

bool checkNorth (int x, int y, string str) {
  //Deal with trival case and avoid array out of bounds/checking silly things
  if (x-strlen(str) < 0){
    return false;
  }
  //for each character in str 
  for (int i = 0; i < (strlen(str)); i++){
    //If the character above in the grid is the next character in the string
    if (grid[x-i][y].letter == str[i]){
      //keep going
      continue;
    }else{
      //It ain't north
      return false;
    }
  }
  //It's north
  return true;
}

Upvotes: 0

Views: 135

Answers (1)

Jonathan Leffler
Jonathan Leffler

Reputation: 754920

Since strlen() returns a value of type size_t, which is unsigned, then you compute x - 3 the computation is done on unsigned values, and with unsigned arithmetic, 0 - 3 is a large positive value, and is never less than zero. In fact, since the comparison is for an unsigned quantity less than 0, but unsigned quantities can never be negative, the compiler can optimize the entire if test and the following return completely away.

You can fix the problem by changing:

if (x-strlen(str) < 0){

to:

if (x < strlen(str)) {

Note that your loop is somewhat inefficient. You have:

for (int i = 0; i < (strlen(str)); i++){

This recomputes strlen(str) on each iteration. You could write:

size_t len = strlen(str);

for (size_t i = 0; i < len; i++) {

This guarantees that the length is only computed once, which is sufficient unless you're modifying the string somewhere in the function, or one of the functions it calls.

Upvotes: 8

Related Questions