Is this a correct code modification?

[Imla]

// stm32f1xx_hal_rtc.c line 827

sTime->Seconds = (uint8_t)((counter_time % 3600U) % 60U);

// to this

sTime->Seconds = (uint8_t)(counter_time % 60U);

[Peter BENSCH]

Welcome, @Imla​, to the community!

This particular case with 3600 and 60 can indeed be shortened to 60, but it cannot be generalised. For example, if you want to shorten ((counter_time % 20U) % 5U) to (counter_time % 4U), it will only fit in 80% of the cases, which is due to the mathematical peculiarity of modulo.

Good luck!

Regards

/Peter

[waclawek.jan]

> ((counter_time % 20U) % 5U) to (counter_time % 4U), it will only fit in 80% of the cases

Only 20% of the cases... ((counter_time % 20U) % 5U) is equivalent to (counter_time % 5U).

JW

[Peter BENSCH]

Oops, yes, I did a mistake and Jan is right:

((X mod Y) mod Z) can be shortened to (X mod Z)

Regards

/Peter

[Imla]

I know the math... But I'm still wondering about the reasons behind that code?! given that the compiler did not optimize the code with a single expression (counter_time % 3600U) % 60U) ...I didn't checked the version where the expression (counter_time % 3600U) used twice at the same code...anyway stm32 has a plenty of SRAM and I must forget Attiny at least for now.

[Danish1]

I wonder if it is an attempt to force the compiler to do the conversion-to-8-bits at a time when it won't garble the result.

I don't know the width of counter-time, but it seems reasonable to assume it is wider than 8 bits.

If an optimiser were to look at code that said (uint8_t)(a <op> b) where b fitted into uint8_t, would it try to convert the code to ((uint8_t)a) <op> b?

Probably not on stm32, but one can imagine that being considered for an 8-bit microcontroller (I'm more familiar with PIC16 than AtTiny), where even 16-bit arithmetic is expensive.

There's a lot of "history" in code. If it's known to work, and is not too inefficient, then is it worth the risk of rewriting it and possibly introducing new bugs?