Interger underflow causing modulo to fail

I have 4 images that I would like to display, so I am using an unsigned short to represent the index of the current image. When I press A, the index decreases by 1; When I press D, the index increases by 1.

I am calculating the index on the A keypress using image_index = (image_index - 1) % 4; (and on the D keypress, image_index = (image_index + 1) % 4;)

Everything works just as expected if I cycle thru going forwards (IE, pressing D), but if I am at index 0 and press A, it underflows to the max value of an unsigned short, and does not modulo by 4 to give me index 3.

I know that for signed types, that overflow/underflow is UB, but I was certain that for unsigned types it was well defined. Any ideas what might be causing it to ignore the modulo?

enum class Errors : short {
  kSdlSuccess = 0,
  kSdlInitFailure = -1,
  kSdlWindowCreationError = -2,
  kSdlRendererCreationError = -3
};

int main(int argc, char** argv) {

  sdl::Context ctx;
  sdl::Window window({ .w = 600, .h = 480, .flags = 0});
  sdl::Renderer renderer(window, {0});

  bool is_running {true};
  unsigned short texture_index = 0;
  SDL_Event event {};
  while(is_running) {
    while(SDL_PollEvent(&event)) {
      if(event.type == SDL_QUIT) { is_running = false; }
      else if(event.type == SDL_KEYDOWN) {
        if(event.key.keysym.scancode == SDL_SCANCODE_A) { texture_index = (texture_index - 1) % 4; }
        else if(event.key.keysym.scancode == SDL_SCANCODE_D) { texture_index = (texture_index + 1) % 4; }
      }
    }

    printf("%d\n", texture_index);

    renderer.setDrawColor(255, 0, 0, 255);
    renderer.clearBuffer();
    renderer.setDrawColor(0,0,0,255);
    renderer.drawTexture(texture_index);
    renderer.swapBuffer();
  }
  return static_cast<int>(Errors::kSdlSuccess);
}

The problem:

That is due to the promotion of unsigned short type to (signed) int. (See this answer for more details).

Let's say that your texture_index is initially zero.

When you do texture_index - 1, your texture_index is promoted to an (signed) int. The result of such calculation is -1 in (signed) int. The result of (-1) % 4 is -1 (Modulo calculation involving negative values can be tricky and counter-intuitive, refer to this question for more details).

Then, you assign (convert) -1 (a signed int) to texture_index (an unsigned short). This conversion yields 65535 (or 0xFFFF). For more details about signed to unsigned conversion, see this answer. So the problem is not of an ignored modulo operation, but of an unwanted type conversion (or promotion).

---

The solution:

So the solution would be eliminating unwanted conversion.

In one comments under this question I saw something like this:

texture_index = (texture_index - 1u) % 4u;

This eliminates the conversion to signed int, great. It still triggers a promotion to unsigned int (since 1u and 4u are unsigned int literals), but since your modulo is small, this doesn't matter.

This works fine in your case but is fragile.

What if one day you want five images?

unsigned short texture_index = 0;
texture_index = (texture_index - 1U) % 5U;
assert(texture_index == 4U);    //Assertion fails!

Why? Debugger now says that texture_index is 0. That is because 0U - 1U == 4294967295U.

One nice trick to bypass such problem is to add the divisor to the dividend before doing modulo.

unsigned short texture_index = 0;
texture_index = (texture_index - 1U + 5U) % 5U;   //Add 5U before modding 5U
assert(texture_index == 4U); //Assertion passes