While implementing STR#1739 I noticed the return value for find_line()
does not document what the return value is if the parameter 'line'
is out of range.
I did an empirical test, writing a program that sends find_line()
values between -10 and 10 on a browser with only 5 items, and found
the return value for the out-of-bounds cases returns NULL (on linux).
This is not immediately apparent from the code (excerpted below).
First, FL_BLINE *l; is uninitialized to 0, and since it is the
value returned, it would SEEM its return value could be wild.
However, this is not the case, because the if/else/else logic
seems to lock the 'l' value down to being either item_first() or
item_last() if the line is out of bounds. But then the two for()
loops seem to undo this, causing 'l' to be NULL in out of bounds
conditions.
So end result is the return value is NULL when line is out of bounds.
I'd like to document this behavior, but I want to make sure
the return value of NULL in these cases is "intentional",
as there are other parts of the code where the return value
of find_line() is not checked for NULL, and could crash.
Here's the implementation of fltk 1.3.x Fl_Browser::find_line()
as it currently reads:
/**
Returns the item for specified \p line.
Note: This call is slow. It's fine for e.g. responding to user
clicks, but slow if called often, such as in a tight sorting loop.
Finding an item 'by line' involves a linear lookup on the internal
linked list. The performance hit can be significant if the browser's
contents is large, and the method is called often (e.g. during a sort).
If you're writing a subclass, use the protected methods item_first(),
item_next(), etc. to access the internal linked list more efficiently.
\param[in] line The line number of the item to return. (1 based)
\returns The returned item.
Returns NULL if line is out of range. /// <-- I WANT TO ADD THIS
\see item_at(), find_line(), lineno()
*/
FL_BLINE* Fl_Browser::find_line(int line) const {
int n; FL_BLINE* l; // UNINITIALIZED/WILD
(INIT'ED BELOW)
if (line == cacheline) return cache;
// FOLLOWING
if/else/else INITIALIZES 'l'
if (cacheline && line > (cacheline/2) && line < ((cacheline+lines)/2)) {
n = cacheline; l = cache;
} else if (line <= (lines/2)) {
n = 1; l = first;
} else {
n = lines; l = last;
}
for (; n < line && l; n++) l = l->next; // <-- LOGIC
DETERMINING NULL RETURN
for (; n > line && l; n--) l = l->prev; // <-- HAPPENS HERE
((Fl_Browser*)this)->cacheline = line;
((Fl_Browser*)this)->cache = l;
return l;
}
_______________________________________________
fltk-dev mailing list
[email protected]
http://lists.easysw.com/mailman/listinfo/fltk-dev
