On Friday 03 July 2009 10:29:02 Johannes Berg wrote: > On Fri, 2009-07-03 at 08:25 +0300, Luciano Coelho wrote: > > If rix is not found in mi->r[], i will become -1 after the loop. This value > > is eventually used to access arrays, so we were accessing arrays with a > > negative index, which is obviously not what we want to do. This patch fixes > > this potential problem. > > This seems odd -- are you or are you not saying that this can happen in > normal operation? > > > @@ -66,7 +66,7 @@ rix_to_ndx(struct minstrel_sta_info *mi, int rix) > > for (i = rix; i >= 0; i--) > > if (mi->r[i].rix == rix) > > break; > > - WARN_ON(mi->r[i].rix != rix); > > + WARN_ON(i < 0); > > return i; > > If it can, this warning seems wrong. Well, the old WARN_ON seems wrong anyway, because it accesses the array out of bounds. In case the loop did not find the entry, the warn on will look like this: WARN_ON(mi->r[-1].rix != rix); So I do think it's correct to replace the WARN_ON with WARN_ON(i < 0), if this can't happen in normal operation. If it can happen in normal op, the warning should be removed and the callers of rix_to_ndx() need to be checked. -- Greetings, Michael. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
