It is interesting why you chose to write a blog article on a diff, without looking at it in context. What is your end game, lol.

:) I am glad to have enterained you.

My end game is to get people to write

  int const delta = (cp[IPOPT_OLEN] - 1);

instead of repeating expressions like this: (cp[IPOPT_OLEN] - 1) and adding variants elsewhere like this: cp[IPOPT_OLEN]. It's confusing and causes bugs. Naming things helps and costs nothing.

There is no bug.

>delta is allowed to be zero, we make no progress (as in the original bug)

It's not zero progress because the for loop increments cp every iteration.

In the original bug delta = -1.

Thank you for the clarification. I did not look at the surrounding code.

However, my point still stands that extracting the complex expression as delta would have helped and possibly avoided the original bug.

  int const delta = (cp[IPOPT_OLEN] - 1);

Further proof is that the original author first commited a broken fix. Again this was caused by the confusion between delta and delta-1, which would have been clear had delta been explicitly named.

This sort of code is just begging to be misunderstood.

- if (cp[IPOPT_OLEN] > 0 && cp[IPOPT_OLEN] < hlen) {

+ if (cp[IPOPT_OLEN] > 0 && (cp[IPOPT_OLEN] - 1) <= hlen) {

The "bug" is from assuming you are parsing a valid packet. The option length field is the length of the option legnth and option data combined. The minimum value it can have is 1 which is when the length of the option data is 0.

cp[IPOPT_OLEN] refers to the option length. Your delta variable is the length of the option data. The confusing part is that in most network protocols length fields do not count the size of the length number.