From aced667f48c29a160aa4e5c0a8df037092b28189 Mon Sep 17 00:00:00 2001 From: Michael Sevakis Date: Mon, 18 Sep 2017 06:00:05 -0400 Subject: [PATCH] Undo hacks to meant to get around string formatting limitations The new vuprintf makes unnecessary workarounds due to formatting limitations. I checked grep output for whatever appeared to fit but it's possible I missed some instances because they weren't so obvious. Also, this means sound settings can dynamically work with any number of decimals rather than the current assumption of one or two. Add an ipow() function to help and take advantage of dynamic field width and precision. Consolidate string formatting of sound settings. Change-Id: I46caf534859dfd1916cd440cd25e5206b192fcd8 --- apps/debug_menu.c | 60 +++++++++++++----------------- apps/gui/option_select.c | 20 ++-------- apps/gui/skin_engine/skin_tokens.c | 21 +++-------- apps/misc.c | 38 +++++++++++-------- apps/misc.h | 2 + apps/plugins/doom/hu_stuff.c | 8 ++-- apps/plugins/doom/wi_stuff.c | 9 ++--- apps/plugins/otp.c | 22 +++++------ apps/recorder/recording.c | 32 +--------------- firmware/general.c | 6 +-- lib/fixedpoint/fixedpoint.c | 29 +++++++++++++++ lib/fixedpoint/fixedpoint.h | 2 + lib/fixedpoint/fixedpoint.make | 3 -- 13 files changed, 110 insertions(+), 142 deletions(-) diff --git a/apps/debug_menu.c b/apps/debug_menu.c index a7dc764523..4af17700c9 100644 --- a/apps/debug_menu.c +++ b/apps/debug_menu.c @@ -1053,9 +1053,10 @@ static bool view_battery(void) st = power_input_status() & (POWER_INPUT_CHARGER | POWER_INPUT_BATTERY); - lcd_putsf(0, line++, "%s%s", - (st & POWER_INPUT_MAIN_CHARGER) ? " Main" : "", - (st & POWER_INPUT_USB_CHARGER) ? " USB" : ""); + + lcd_putsf(0, line++, "%.*s%.*s", + !!(st & POWER_INPUT_MAIN_CHARGER)*5, " Main", + !!(st & POWER_INPUT_USB_CHARGER)*4, " USB"); y = ARRAYLEN(chrgstate_strings) - 1; @@ -1084,22 +1085,18 @@ static bool view_battery(void) y / 1000, y % 1000); y = battery_adc_charge_current(); - if (y < 0) x = '-', y = -y; - else x = ' '; - lcd_putsf(0, line++, "CHRGISN:%c%d mA", x, y); + lcd_putsf(0, line++, "CHRGISN:% d mA", y); y = cccv_regulator_dissipation(); lcd_putsf(0, line++, "P CCCV : %d mW", y); y = battery_charge_current(); - if (y < 0) x = '-', y = -y; - else x = ' '; - lcd_putsf(0, line++, "I Charge:%c%d mA", x, y); + lcd_putsf(0, line++, "I Charge:% d mA", y); y = battery_adc_temp(); if (y != INT_MIN) { - lcd_putsf(0, line++, "T Battery: %dC (%dF)", y, + lcd_putsf(0, line++, "T Battery: %d\u00b0C (%d\u00b0F)", y, (9*y + 160) / 5); } else { /* Conversion disabled */ @@ -1371,6 +1368,8 @@ static int disk_callback(int btn, struct gui_synclist *lists) #elif (CONFIG_STORAGE & STORAGE_ATA) static int disk_callback(int btn, struct gui_synclist *lists) { + static const char atanums[] = { " 0 1 2 3 4 5 6" }; + (void)lists; int i; char buf[128]; @@ -1414,12 +1413,10 @@ static int disk_callback(int btn, struct gui_synclist *lists) "Read-ahead: %s", i ? "enabled" : "unsupported"); timing_info_present = identify_info[53] & (1<<1); if(timing_info_present) { - char pio3[2], pio4[2];pio3[1] = 0; - pio4[1] = 0; - pio3[0] = (identify_info[64] & (1<<0)) ? '3' : 0; - pio4[0] = (identify_info[64] & (1<<1)) ? '4' : 0; simplelist_addline( - "PIO modes: 0 1 2 %s %s", pio3, pio4); + "PIO modes: 0 1 2%.*s%.*s", + (identify_info[64] & (1<<0)) << 1, &atanums[3*2], + (identify_info[64] & (1<<1)) , &atanums[4*2]); } else { simplelist_addline( @@ -1442,13 +1439,11 @@ static int disk_callback(int btn, struct gui_synclist *lists) "Physical sector size: %d", sector_size); #ifdef HAVE_ATA_DMA if (identify_info[63] & (1<<0)) { - char mdma0[2], mdma1[2], mdma2[2]; - mdma0[1] = mdma1[1] = mdma2[1] = 0; - mdma0[0] = (identify_info[63] & (1<<0)) ? '0' : 0; - mdma1[0] = (identify_info[63] & (1<<1)) ? '1' : 0; - mdma2[0] = (identify_info[63] & (1<<2)) ? '2' : 0; simplelist_addline( - "MDMA modes: %s %s %s", mdma0, mdma1, mdma2); + "MDMA modes:%.*s%.*s%.*s", + (identify_info[63] & (1<<0)) << 1, &atanums[0*2], + (identify_info[63] & (1<<1)) , &atanums[1*2], + (identify_info[63] & (1<<2)) >> 1, &atanums[2*2]); simplelist_addline( "MDMA Cycle times %dns/%dns", identify_info[65], @@ -1459,18 +1454,15 @@ static int disk_callback(int btn, struct gui_synclist *lists) "No MDMA mode info"); } if (identify_info[53] & (1<<2)) { - char udma0[2], udma1[2], udma2[2], udma3[2], udma4[2], udma5[2], udma6[2]; - udma0[1] = udma1[1] = udma2[1] = udma3[1] = udma4[1] = udma5[1] = udma6[1] = 0; - udma0[0] = (identify_info[88] & (1<<0)) ? '0' : 0; - udma1[0] = (identify_info[88] & (1<<1)) ? '1' : 0; - udma2[0] = (identify_info[88] & (1<<2)) ? '2' : 0; - udma3[0] = (identify_info[88] & (1<<3)) ? '3' : 0; - udma4[0] = (identify_info[88] & (1<<4)) ? '4' : 0; - udma5[0] = (identify_info[88] & (1<<5)) ? '5' : 0; - udma6[0] = (identify_info[88] & (1<<6)) ? '6' : 0; simplelist_addline( - "UDMA modes: %s %s %s %s %s %s %s", udma0, udma1, udma2, - udma3, udma4, udma5, udma6); + "UDMA modes:%.*s%.*s%.*s%.*s%.*s%.*s%.*s", + (identify_info[88] & (1<<0)) << 1, &atanums[0*2], + (identify_info[88] & (1<<1)) , &atanums[1*2], + (identify_info[88] & (1<<2)) >> 1, &atanums[2*2], + (identify_info[88] & (1<<3)) >> 2, &atanums[3*2], + (identify_info[88] & (1<<4)) >> 3, &atanums[4*2], + (identify_info[88] & (1<<5)) >> 4, &atanums[5*2], + (identify_info[88] & (1<<6)) >> 5, &atanums[6*2]); } else { simplelist_addline( @@ -1691,7 +1683,7 @@ static int ata_smart_callback(int btn, struct gui_synclist *lists) } else { - simplelist_addline("ATA SMART error: 0x%x", rc); + simplelist_addline("ATA SMART error: %#x", rc); } read_done = true; } @@ -2165,7 +2157,7 @@ static int radio_callback(int btn, struct gui_synclist *lists) tuner_get_rds_info(RADIO_RDS_NAME, buf, sizeof (buf)); tuner_get_rds_info(RADIO_RDS_PROGRAM_INFO, &pi, sizeof (pi)); - simplelist_addline("PI:%04X PS:'%8s'", pi, buf); + simplelist_addline("PI:%04X PS:'%-8s'", pi, buf); tuner_get_rds_info(RADIO_RDS_TEXT, buf, sizeof (buf)); simplelist_addline("RT:%s", buf); tuner_get_rds_info(RADIO_RDS_CURRENT_TIME, &seconds, sizeof (seconds)); diff --git a/apps/gui/option_select.c b/apps/gui/option_select.c index 0452467994..f99e833a1e 100644 --- a/apps/gui/option_select.c +++ b/apps/gui/option_select.c @@ -106,23 +106,9 @@ const char *option_get_valuestring(const struct settings_list *setting, } else if ((setting->flags & F_T_SOUND) == F_T_SOUND) { - char sign = ' '; - const char *unit = sound_unit(setting->sound_setting->setting); - int val = sound_val2phys(setting->sound_setting->setting, (int)temp_var); - if (sound_numdecimals(setting->sound_setting->setting)) - { - int integer, dec; - if(val < 0) - { - sign = '-'; - val = abs(val); - } - integer = val / 10; - dec = val % 10; - snprintf(buffer, buf_len, "%c%d.%d %s", sign, integer, dec, unit); - } - else - snprintf(buffer, buf_len, "%d %s", val, unit); + format_sound_value(buffer, buf_len, + setting->sound_setting->setting, + temp_var); } else if ((setting->flags & F_CHOICE_SETTING) == F_CHOICE_SETTING) { diff --git a/apps/gui/skin_engine/skin_tokens.c b/apps/gui/skin_engine/skin_tokens.c index cbf732fe10..1cff83eb9a 100644 --- a/apps/gui/skin_engine/skin_tokens.c +++ b/apps/gui/skin_engine/skin_tokens.c @@ -70,6 +70,7 @@ #if CONFIG_TUNER #include "radio.h" #include "tuner.h" +#include "fixedpoint.h" #endif #include "list.h" @@ -432,23 +433,11 @@ const char *get_id3_token(struct wps_token *token, struct mp3entry *id3, /* Returns buf */ static char *format_freq_MHz(int freq, int freq_step, char *buf, int buf_size) { - int scale, div; - char *fmt; - if (freq_step < 100000) - { - /* Format with two digits after decimal point */ - scale = 10000; - fmt = "%d.%02d"; - } - else - { - /* Format with one digit after decimal point */ - scale = 100000; - fmt = "%d.%d"; - } - div = 1000000 / scale; + int decimals = (freq_step < 100000) + 1; + int scale = ipow(10, 6 - decimals); + int div = 1000000 / scale; freq = freq / scale; - snprintf(buf, buf_size, fmt, freq/div, freq%div); + snprintf(buf, buf_size, "%d.%.*d", freq/div, decimals, freq%div); return buf; } diff --git a/apps/misc.c b/apps/misc.c index 3fdcab85e6..bfe3e990f5 100644 --- a/apps/misc.c +++ b/apps/misc.c @@ -62,6 +62,7 @@ #include "yesno.h" #include "viewport.h" #include "list.h" +#include "fixedpoint.h" #include "debug.h" @@ -1060,21 +1061,13 @@ char* skip_whitespace(char* const str) */ void format_time(char* buf, int buf_size, long t) { - int const time = abs(t / 1000); - int const hours = time / 3600; - int const minutes = time / 60 - hours * 60; - int const seconds = time % 60; - const char * const sign = &"-"[t < 0 ? 0 : 1]; - - if ( hours == 0 ) - { - snprintf(buf, buf_size, "%s%d:%02d", sign, minutes, seconds); - } - else - { - snprintf(buf, buf_size, "%s%d:%02d:%02d", sign, hours, minutes, - seconds); - } + unsigned long time = labs(t / 1000); + unsigned long hours = time / 3600; + unsigned long minutes = time / 60 - hours * 60; + unsigned long seconds = time % 60; + int hashours = hours > 0; + snprintf(buf, buf_size, "%.*s%.0lu%.*s%.*lu:%.2lu", + t < 0, "-", hours, hashours, ":", hashours+1, minutes, seconds); } /** @@ -1260,3 +1253,18 @@ enum current_activity get_current_activity(void) } #endif + +/* format a sound value like: -1.05 dB */ +int format_sound_value(char *buf, size_t size, int snd, int val) +{ + int numdec = sound_numdecimals(snd); + const char *unit = sound_unit(snd); + int physval = sound_val2phys(snd, val); + + unsigned int factor = ipow(10, numdec); + unsigned int av = abs(physval); + unsigned int i = av / factor; + unsigned int d = av - i*factor; + return snprintf(buf, size, "%c%u%.*s%.*u %s", " -"[physval < 0], + i, numdec, ".", numdec, d, unit); +} diff --git a/apps/misc.h b/apps/misc.h index 0d4cba6cd3..b13c0b15c6 100644 --- a/apps/misc.h +++ b/apps/misc.h @@ -167,5 +167,7 @@ void push_current_activity(enum current_activity screen); void pop_current_activity(void); enum current_activity get_current_activity(void); +/* format a sound value like: -1.05 dB */ +int format_sound_value(char *buf, size_t len, int snd, int val); #endif /* MISC_H */ diff --git a/apps/plugins/doom/hu_stuff.c b/apps/plugins/doom/hu_stuff.c index 639c963f72..6a1b07b238 100644 --- a/apps/plugins/doom/hu_stuff.c +++ b/apps/plugins/doom/hu_stuff.c @@ -311,14 +311,14 @@ void HU_Init(void) { snprintf(buffer, sizeof(buffer), "DIG%d",j-48); R_SetPatchNum(hu_font2 +i, buffer); - snprintf(buffer, sizeof(buffer), "STCFN%s%d", (j/10>0?"0":"00"), j); //NOTE ROCKHACK: "STCFN%.3d" + snprintf(buffer, sizeof(buffer), "STCFN%.3d", j); R_SetPatchNum(&hu_font[i], buffer); } else if ('A'<=j && j<='Z') { snprintf(buffer, sizeof(buffer), "DIG%c",j); R_SetPatchNum(hu_font2 +i, buffer); - snprintf(buffer, sizeof(buffer), "STCFN%s%d", (j/10>0?"0":"00"), j); //NOTE ROCKHACK: "STCFN%.3d" + snprintf(buffer, sizeof(buffer), "STCFN%.3d", j); R_SetPatchNum(&hu_font[i], buffer); } else if (j=='-') @@ -348,14 +348,14 @@ void HU_Init(void) } else if (j<97) { - snprintf(buffer, sizeof(buffer), "STCFN%s%d", (j/10>0?"0":"00"), j); //NOTE ROCKHACK: "STCFN%.3d" + snprintf(buffer, sizeof(buffer), "STCFN%.3d", j); R_SetPatchNum(hu_font2 +i, buffer); R_SetPatchNum(&hu_font[i], buffer); //jff 2/23/98 make all font chars defined, useful or not } else if (j>122) { - snprintf(buffer, sizeof(buffer), "STBR%d", j); //NOTE: "STBR%.3d" + snprintf(buffer, sizeof(buffer), "STBR%.3d", j); R_SetPatchNum(hu_font2 +i, buffer); R_SetPatchNum(&hu_font[i], buffer); } diff --git a/apps/plugins/doom/wi_stuff.c b/apps/plugins/doom/wi_stuff.c index 7c7831d084..b73839f55b 100644 --- a/apps/plugins/doom/wi_stuff.c +++ b/apps/plugins/doom/wi_stuff.c @@ -397,8 +397,7 @@ void WI_unloadData(void); void WI_levelNameLump(int epis, int map, char* buf, int bsize) { if (gamemode == commercial) { - snprintf(buf, bsize,"CWILV%s%d",(map/10>0?"":"0"), map); //ANOTHER ROCKHACK "CWILV%2.2d" - //snprintf(buf,bsize, "CWILV%2.2d", map); + snprintf(buf,bsize, "CWILV%2.2d", map); } else { snprintf(buf,bsize, "WILV%d%d", epis, map); } @@ -1829,8 +1828,7 @@ void WI_loadData(void) if (wbs->epsd != 1 || j != 8) { // animations - snprintf(name, sizeof(name),"WIA%d%s%d%s%d", wbs->epsd, (j/10>0?"":"0"), j,(i/10>0?"":"0"), i); //ANOTHER ROCKHACK - //snprintf(name, sizeof(name),"WIA%d%.2d%.2d", wbs->epsd, j, i); + snprintf(name, sizeof(name),"WIA%d%.2d%.2d", wbs->epsd, j, i); a->p[i] = W_CacheLumpName(name); } else @@ -1872,8 +1870,7 @@ void WI_unloadData(void) // MONDO HACK! if (wbs->epsd != 1 || j != 8) { // animations - snprintf(name, sizeof(name),"WIA%d%s%d%s%d", wbs->epsd, (j/10>0?"":"0"), j,(i/10>0?"":"0"), i); //ANOTHER ROCKHACK - //snprintf(name,sizeof(name), "WIA%d%.2d%.2d", wbs->epsd, j, i); + snprintf(name,sizeof(name), "WIA%d%.2d%.2d", wbs->epsd, j, i); W_UnlockLumpName(name); } } diff --git a/apps/plugins/otp.c b/apps/plugins/otp.c index 69cb8b7982..6dece4ad38 100644 --- a/apps/plugins/otp.c +++ b/apps/plugins/otp.c @@ -516,25 +516,23 @@ static void add_acct(void) static void show_code(int acct) { - /* rockbox's printf doesn't support a variable field width afaik */ - char format_buf[64]; if(!accounts[acct].is_totp) { - rb->snprintf(format_buf, sizeof(format_buf), "%%0%dd", accounts[acct].digits); - rb->splashf(0, format_buf, HOTP(accounts[acct].secret, - accounts[acct].sec_len, - accounts[acct].hotp_counter, - accounts[acct].digits)); + rb->splashf(0, "%0*d", accounts[acct].digits, + HOTP(accounts[acct].secret, + accounts[acct].sec_len, + accounts[acct].hotp_counter, + accounts[acct].digits)); ++accounts[acct].hotp_counter; } #if CONFIG_RTC else { - rb->snprintf(format_buf, sizeof(format_buf), "%%0%dd (%%ld second(s) left)", accounts[acct].digits); - rb->splashf(0, format_buf, TOTP(accounts[acct].secret, - accounts[acct].sec_len, - accounts[acct].totp_period, - accounts[acct].digits), + rb->splashf(0, "%0*d (%ld seconds(s) left)", accounts[acct].digits, + TOTP(accounts[acct].secret, + accounts[acct].sec_len, + accounts[acct].totp_period, + accounts[acct].digits), accounts[acct].totp_period - get_utc() % accounts[acct].totp_period); } #else diff --git a/apps/recorder/recording.c b/apps/recorder/recording.c index 4816b3bad4..7357b469f4 100644 --- a/apps/recorder/recording.c +++ b/apps/recorder/recording.c @@ -548,39 +548,9 @@ static void auto_gain_control(int *peak_l, int *peak_r, int *balance) } #endif /* HAVE_AGC */ -static const char* const fmtstr[] = -{ - "%c%d %s", /* no decimals */ - "%c%d.%d %s ", /* 1 decimal */ - "%c%d.%02d %s " /* 2 decimals */ -}; - -static const char factor[] = {1, 10, 100}; - static char *fmt_gain(int snd, int val, char *str, int len) { - int i, d, numdec; - const char *unit; - char sign = ' '; - - val = sound_val2phys(snd, val); - if(val < 0) - { - sign = '-'; - val = -val; - } - numdec = sound_numdecimals(snd); - unit = sound_unit(snd); - - if(numdec) - { - i = val / factor[numdec]; - d = val % factor[numdec]; - snprintf(str, len, fmtstr[numdec], sign, i, d, unit); - } - else - snprintf(str, len, fmtstr[numdec], sign, val, unit); - + format_sound_value(str, len, snd, val); return str; } diff --git a/firmware/general.c b/firmware/general.c index c70d21c4a0..a4e7fba0f6 100644 --- a/firmware/general.c +++ b/firmware/general.c @@ -107,7 +107,6 @@ char *create_numbered_filename(char *buffer, const char *path, int pathlen; int prefixlen = strlen(prefix); int suffixlen = strlen(suffix); - char fmtstring[12]; if (buffer != path) strlcpy(buffer, path, MAX_PATH); @@ -152,9 +151,8 @@ char *create_numbered_filename(char *buffer, const char *path, max_num++; - snprintf(fmtstring, sizeof(fmtstring), "/%%s%%0%dd%%s", numberlen); - snprintf(buffer + pathlen, MAX_PATH - pathlen, fmtstring, prefix, - max_num, suffix); + snprintf(buffer + pathlen, MAX_PATH - pathlen, "/%s%0*d%s", prefix, + numberlen, max_num, suffix); #ifdef IF_CNFN_NUM if (num) diff --git a/lib/fixedpoint/fixedpoint.c b/lib/fixedpoint/fixedpoint.c index 645419d102..d1307bb248 100644 --- a/lib/fixedpoint/fixedpoint.c +++ b/lib/fixedpoint/fixedpoint.c @@ -211,6 +211,35 @@ long fp_sqrt(long x, unsigned int fracbits) return g; } +/* raise an integer to an integer power */ +long ipow(long x, long y) +{ + /* y[k] = bit k of y, 0 or 1; k=0...n; n=|_ lg(y) _| + * + * x^y = x^(y[0]*2^0 + y[1]*2^1 + ... + y[n]*2^n) + * = x^(y[0]*2^0) * x^(y[1]*2^1) * ... * x^(y[n]*2^n) + */ + long a = 1; + + if (y < 0 && x != -1) + { + a = 0; /* would be < 1 or +inf if x == 0 */ + } + else + { + while (y) + { + if (y & 1) + a *= x; + + y /= 2; + x *= x; + } + } + + return a; +} + /** * Fixed point sinus using a lookup table * don't forget to divide the result by 16384 to get the actual sinus value diff --git a/lib/fixedpoint/fixedpoint.h b/lib/fixedpoint/fixedpoint.h index bc50ff687d..dcd7c8298c 100644 --- a/lib/fixedpoint/fixedpoint.h +++ b/lib/fixedpoint/fixedpoint.h @@ -85,6 +85,8 @@ long fp14_sin(int val); long fp16_log(int x); long fp16_exp(int x); +long ipow(long x, long y); + /* fast unsigned multiplication (16x16bit->32bit or 32x32bit->32bit, * whichever is faster for the architecture) */ #ifdef CPU_ARM diff --git a/lib/fixedpoint/fixedpoint.make b/lib/fixedpoint/fixedpoint.make index 0233e9499b..5be0e38ea7 100644 --- a/lib/fixedpoint/fixedpoint.make +++ b/lib/fixedpoint/fixedpoint.make @@ -13,11 +13,8 @@ FIXEDPOINTLIB_OBJ := $(call c2obj, $(FIXEDPOINTLIB_SRC)) INCLUDES += -I$(FIXEDPOINTLIB_DIR) OTHER_SRC += $(FIXEDPOINTLIB_SRC) -# If not SOFTWARECODECS, then only plugins depend upon us -ifdef SOFTWARECODECS CORE_LIBS += $(FIXEDPOINTLIB) CORE_GCSECTIONS := yes -endif FIXEDPOINTLIB_FLAGS := $(CFLAGS) $(SHARED_CFLAGS)