Confusion with variable update in microcontroller programming

Question

I use an STM32 microcontroller board and to summarize the question, in the following code inside the if clause the variable called current_val does not take the value it is assigned some lines before:

int main(void) {
  //initialization code
  while (1) {
    uint16_t I2C_value = get_data();

    float current_val = (I2C_value / 4096.0 - 0.5) * 50 * 1000;
    sprintf(msg_int, "%.3f \r\n", current_val);
    HAL_UART_Transmit(&huart2, (uint8_t*) msg_int, strlen(msg_int), HAL_MAX_DELAY);

    HAL_Delay(250);
        
    if (current_val > 500) {
      HAL_GPIO_WritePin(GPIOA, GPIO_PIN_10, GPIO_PIN_SET);
    }
    else {
      HAL_GPIO_WritePin(GPIOA, GPIO_PIN_10, GPIO_PIN_RESET);
    }
  }
}

But if I declare current_val as global it works:

float current_val;

int main(void) {
  //initialization code
  while (1) {
    uint16_t I2C_value = averaging_data();

    current_val = (I2C_value / 4096.0 - 0.5) * 50 * 1000;
    sprintf(msg_int, "%.3f \r\n", current_val);
    HAL_UART_Transmit(&huart2, (uint8_t*) msg_int, strlen(msg_int), HAL_MAX_DELAY);

    HAL_Delay(250);

    if (current_val > 500) {
      HAL_GPIO_WritePin(GPIOA, GPIO_PIN_10, GPIO_PIN_SET);
    }
    else {
      HAL_GPIO_WritePin(GPIOA, GPIO_PIN_10, GPIO_PIN_RESET);
    }
  }
}

What could be the reason in the first case current_val is not remaining the same after it is assigned? I don't understand what makes it change since there is nothing in the code to update its value.

current_val becomes zero in the first case. I dont think, this is what happened. — user1245, Jun 08 '22 at 16:37
And what are the values you are expecting it to get? I suspect you are overflowing it. Don't forget, it needs a space for a null-terminator as well. — Eugene Sh., Jun 08 '22 at 16:44
If it is around `500`, (as you are comparing to it), it will be `"499.123 \r\n"` - which is already 10 characters, with no place for the terminator. — Eugene Sh., Jun 08 '22 at 16:46
@EugeneSh. float current_val should be between -12 to 700 in my case. In the first case HAL_UART_Transmit outputs correct but after that when it comes to if the circuit didnt work so I found out current_val becomes zero. So it doesnt hold the value somehow, — user1245, Jun 08 '22 at 16:46
As I said, if it is over 100, then your program has an undefined behavior, as your string is overflown — Eugene Sh., Jun 08 '22 at 16:47
@EugeneSh. But why doesn't that happen in the second case? Just curicous. — user1245, Jun 08 '22 at 16:48
Because as they (we) are saying on StackOverflow - undefined behavior is undefined :) In practice you have a different memory layout. So in one, the buffer overrun is affecting this value, in the other it is not. You might have some other issues though. But this is a definite one. Increase the buffer size sparingly. — Eugene Sh., Jun 08 '22 at 16:48
@EugeneSh. Interesting. I dont have the board with me now. But I will definitely update you here after testing it tomorrow. — user1245, Jun 08 '22 at 16:51
@bobflux Won't help much. It will swallow the null-terminator, and then the `strlen` will go crazy. — Eugene Sh., Jun 08 '22 at 16:52
@bobflux it is not only printing issue it also really changes the value of the variable. Since the if condition doesnt work as expected. — user1245, Jun 08 '22 at 16:53
@EugeneSh. snprintf() always adds the null unless you give it a zero byte buffer — bobflux, Jun 08 '22 at 16:59
@user1245 sprintf() is writting stuff after the end of the buffer, when something writes bytes in memory where they're not supposed to be, stuff can happen. If it overwrites your variable, then it'll have a different value afterwards, of course — bobflux, Jun 08 '22 at 17:00
@bobflux Right. Mixed with the other `*n*` functions, such as `strncpy`, which are not so courteous. — Eugene Sh., Jun 08 '22 at 17:11
Morale of the story is rather: don't use PC libs like stdio.h in an embedded system. Writing some manner of serial bus or display driver isn't rocket science. — Lundin, Jun 14 '22 at 13:02

score 3 · Answer 1 · answered Jun 08 '22 at 17:42

As pointed out by @EugeneSh, your sprintf line will overflow the msg_int buffer whenever current_val reaches a value of 100.
You would have 3 digits, a dot, another 3 digits, a space, and ending with a CR-LF pair, for a total of 10 characters. But since a C string is always terminated with a \0 or "null" character, you actually need to allocate 11 bytes for your msg_int array, not 10.

In your first case, both current_val and msg_int are local variables on the stack of your main function, and will probably be allocated memory space next to each other.
So when your sprintf writes to msg_int, and current_val >= 100, it could end up overwriting the 1st byte of current_val with a 0 (the string terminator).

In your second case, although nothing has changed with the snprintf call overflowing msg_int, now that current_val is a global variable it gets a statically assigned memory location so it's no longer on the stack next to msg_int - so even though the overflow bug is still present it doesn't overwrite current_val, but overwrites something else instead (which may come back to bite you at some point in the future).

The solution to this is clearly to allocate an extra byte to msg_int, but what you should also do in future is never use sprintf to write a string to a buffer, but rather use snprintf instead.
The difference between them being that you give snprintf the size of the buffer it's writing to so that it wont ever overflow it.

Confusion with variable update in microcontroller programming

1 Answers1