[Geany-Devel] [Geany-devel] Code review: suggest reorder of condition to put bounds check before use

[Colomban Wendling]

…and 2 years later :

Le 06/08/2011 16:03, Daniel Marjamäki a écrit :
> Hi!

Hey,

> I saw this code in src/symbols.c at line 1917:
> 
>        while (sci_get_style_at(sci, start) != fn_style
>                && start < max_pos) start++;
> 
> If start >= max_pos then sci_get_style_at will be called (with out of
> bounds value?) and then the loop will bail out.

The "out of bound" isn't really an issue since 1) it's probably nor out
of document's bounds, just pass what we want to check, and 2)
sci_get_style_at() handles invalid positions gracefully.

> I suggest that the condition is reordered as:
> 
>        while (start < max_pos
>                && sci_get_style_at(sci, start) != fn_style)
>                start++;
> 
> Then sci_get_style_at will only be called if start is less than max_pos.
> 
> It is just my humble suggestion.

Good catch, it'd be better swapped.  Changed with
https://github.com/geany/geany/commit/6e902613b38372486b831be121af7579f377a7c5
thanks :)

Cheers,
Colomban