| Message ID | ddfb44ed924615bdb61a30ae7627326942575567.1743073557.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Range-check array index before access | expand |
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > Before accessing an array element at a given index, we should make sure > that the index is within the desired bounds, not afterwards, otherwise > it may not make sense to even access the array element in the first > place. > > Pointed out by CodeQL's `cpp/offset-use-before-range-check` rule. At least this part should say this is a false positive, forcing us to make an unnecessary change to help future developers who are running "git blame" and "git log -p" to find out why only s[off] checked against CR needs this check _before_ it, while checking against other values needs _no_ check. In other words, the first paragraph of the proposed log message is a total red herring. We are accessing an array element at a given index 'off' in the original, we are still accessing the same element in the updated code, and we know the index is within the array bounds. If the condition were "We want to skip CR only at odd places", we would have written || (s[off] == '\r' && (off & 01)) or || ((off & 01) || s[off] == '\r') and both are equally valid. (off < len -1) should be no different. > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > --- > diff.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/diff.c b/diff.c > index c89c15d98e0..18ba3060460 100644 > --- a/diff.c > +++ b/diff.c > @@ -892,7 +892,7 @@ static void fill_es_indent_data(struct emitted_diff_symbol *es) > > /* skip any \v \f \r at start of indentation */ > while (s[off] == '\f' || s[off] == '\v' || > - (s[off] == '\r' && off < len - 1)) > + (off < len - 1 && s[off] == '\r')) > off++; > > /* calculate the visual width of indentation */
diff --git a/diff.c b/diff.c index c89c15d98e0..18ba3060460 100644 --- a/diff.c +++ b/diff.c @@ -892,7 +892,7 @@ static void fill_es_indent_data(struct emitted_diff_symbol *es) /* skip any \v \f \r at start of indentation */ while (s[off] == '\f' || s[off] == '\v' || - (s[off] == '\r' && off < len - 1)) + (off < len - 1 && s[off] == '\r')) off++; /* calculate the visual width of indentation */