Why doesn't my code work when replacing 622.08E6 with 622080000?

Question

I recently came across a C code (working by the way) where I found

freq_xtal = ((622.08E6 * vcxo_reg_val->hiv * vcxo_reg_val->n1)/(temp_rfreq));

From my intuition it seems that 622.08E6 should mean 622.08 x 10⁶. From this question this assumption is correct.

So I tried replacing 622.08e6 with

uint32_t default_freq = 622080000;

For some reason this doesn't seem to work

Any thoughts or suggestions appreciated

What does not work, do you receive and error are your values wrong? — Shafik Yaghmour, Nov 14 '13 at 03:34
The chip I am trying to program does not behave as expected with just this one change. — liv2hak, Nov 14 '13 at 03:34
We don't have enough information, some code would probably help, it is suspicious that you are using a integer value in place of a floating literal. — Shafik Yaghmour, Nov 14 '13 at 03:38

score 6 · Answer 1 · answered Nov 14 '13 at 03:37

6

The problem you are having (and I'm speculating here because I don't have the rest of your code) appears to be that replacing the floating point with an integer caused the multiplication and division to be integer based, and not decimal based. As a result, you now compute the wrong value.

Try type casting your uint32_t to a double and see if that clears it up.

answered Nov 14 '13 at 03:37

It'sPete

5,083
8
39
72

the main problem here is overflow and not integer math, because 622080000 is already very big – phuclv Jul 20 '18 at 17:28

score 1 · Answer 2 · edited Jun 20 '20 at 09:12

The problem is due to overflow!

The original expression (622.08E6 * vcxo_reg_val->hiv * vcxo_reg_val->n1)/temp_rfreq (you have too many unnecessary parentheses though) is done in double precision because 622.08E6 is a double literal. That'll result in a floating-point value

However if you replace the literal with 622080000 then the whole expression will be done in integer math if all the variables are integer. But more importantly, integer math will overflow (at least much sooner than floating-point one)

Notice that UINT32_MAX / 622080000.0 ≈ 6.9. That means just multiply the constant by 7 and it'll overflow. However in the code you multiply 622080000 with 2 other values whose product may well be above 6. You should add the ULL suffix to do the math in unsigned long long

freq_xtal = (622080000ULL * vcxo_reg_val->hiv * vcxo_reg_val->n1)/temp_rfreq;

or change the variable to uint64_t default_freq = 622080000ULL;

Why doesn't my code work when replacing 622.08E6 with 622080000?

2 Answers2

The problem is due to overflow!