Search code examples
cheap-memoryheap-corruptionpebble-watchcloudpebble

Pebble Heap Corruption With Str(n)cat


When updating my battery display for my Pebble watchface, I get the battery level into int s_battery_level, run it through my toString function (which works 100% of the time, I've tested it), and then append a % and a null terminator, just for good measure. However, for some reason, when that line is run, I get a heap corruption error when the battery drops from 100% to 90% (or 100 to any lower-digit number). I've tried removing/adding in the null terminator, used both strcat() and strncat(), even escaping the %, and nothing has seemed to work. Once I remove the offending line, it works perfectly.

Why is this line corrupting the heap? And optionally, how could I fix/avoid it?

static void battery_update_proc(Layer *layer, GContext *ctx) {
  graphics_context_set_text_color(ctx, TxtColor);
  char * lvl = calloc(6,sizeof(char));
  lvl = toString(s_battery_level, lvl);
  //strncat(lvl, "%\0", 2);
  //offending line above ^
  APP_LOG(APP_LOG_LEVEL_DEBUG, "%s", lvl);
  graphics_draw_text(ctx, lvl, s_txt_font, GRect(bound.origin.x + ROUND_OFFSET_BA_X, bound.size.h * .5 + ROUND_OFFSET_BA_Y, bound.size.w, 24), GTextOverflowModeWordWrap, GTextAlignmentCenter, NULL);
}

And for good measure, here is the toString function:

char * toString(int value, char * result) {
    int digit = digits(value);
    result = calloc(digit + 1, sizeof(char));
    result[digit] = '\0';
    int usedVal = 0;
    for (int i = digit; i > 0; i--)
    {
        int x = (value - usedVal) / pwrOf10(i - 1);
        result[digit - i] = (char) x + '0';
        usedVal = usedVal + (result[digit - i] - '0') * pwrOf10(i - 1);
    }
    return result;
}

int digits(int n) {
    if (n < 0) return digits((n == 0) ? 9999 : -n);
    if (n < 10) return 1;
    return 1 + digits(n / 10);
}

int pwrOf10(int power) {
    int val = 1;
    int i = power;
    while (i > 0)
    {
        val *= 10;
        i--;
    }
return val;
}

Solution

  • You're allocating memory for lvl twice - once in battery_update_proc and then again in toString. It's the second time that is the issue as it's allocating only enough space for the digits.

    And you're then not freeing either chunk of memory, so you've got a memory leak too.

    As to how to fix it, why not just replace the call to toString with sprintf(lvl,"%d%%",s_battery_level) and then remember to call free(lvl) at the end of the function.