Hi Tom, On Thu, Jan 14, 2016 at 01:02:03PM -0500, Tom Rini wrote: >Enabling this function always removes some class of string saftey issues. >The size change here in general is about 400 bytes and this seems a reasonable >trade-off. > >Cc: Peng Fan <peng....@nxp.com> >Cc: Peter Robinson <pbrobin...@gmail.com> >Cc: Fabio Estevam <fabio.este...@freescale.com> >Cc: Adrian Alonso <aalo...@freescale.com> >Cc: Stefano Babic <sba...@denx.de> >Cc: Hans de Goede <hdego...@redhat.com> >Signed-off-by: Tom Rini <tr...@konsulko.com>
Reviewed-and-tested-by: Peng Fan <peng....@nxp.com> Regards, Peng. > >--- >I ran my usual world build of buildman with size comparison options this time >to test this out. The only exception to the general size change here are >q8_a23_tablet_800x480 and gt90h_v4 both in the non-SPL case. These both grow >by 3780 bytes in the non-SPL case. In all cases except those two, when paired >with the gzwrite patch I posted yesterday we gain all of that back, or more. >--- > README | 9 --------- > configs/bayleybay_defconfig | 1 - > configs/chromebook_link_defconfig | 1 - > configs/chromebox_panther_defconfig | 1 - > configs/coreboot-x86_defconfig | 1 - > configs/crownbay_defconfig | 1 - > configs/galileo_defconfig | 1 - > configs/minnowmax_defconfig | 1 - > configs/qemu-x86_defconfig | 1 - > configs/sandbox_defconfig | 1 - > include/vsprintf.h | 12 ------------ > lib/Kconfig | 9 --------- > lib/vsprintf.c | 12 ------------ > 13 files changed, 51 deletions(-) > >diff --git a/README b/README >index 5ac2d44..b7ce63d 100644 >--- a/README >+++ b/README >@@ -890,15 +890,6 @@ The following options need to be configured: > 'Sane' compilers will generate smaller code if > CONFIG_PRE_CON_BUF_SZ is a power of 2 > >-- Safe printf() functions >- Define CONFIG_SYS_VSNPRINTF to compile in safe versions of >- the printf() functions. These are defined in >- include/vsprintf.h and include snprintf(), vsnprintf() and >- so on. Code size increase is approximately 300-500 bytes. >- If this option is not given then these functions will >- silently discard their buffer size argument - this means >- you are not getting any overflow checking in this case. >- > - Boot Delay: CONFIG_BOOTDELAY - in seconds > Delay before automatically booting the default image; > set to -1 to disable autoboot. >diff --git a/configs/bayleybay_defconfig b/configs/bayleybay_defconfig >index f462e05..0879d1e 100644 >--- a/configs/bayleybay_defconfig >+++ b/configs/bayleybay_defconfig >@@ -37,4 +37,3 @@ CONFIG_VIDEO_VESA=y > CONFIG_FRAMEBUFFER_SET_VESA_MODE=y > CONFIG_FRAMEBUFFER_VESA_MODE_11A=y > CONFIG_USE_PRIVATE_LIBGCC=y >-CONFIG_SYS_VSNPRINTF=y >diff --git a/configs/chromebook_link_defconfig >b/configs/chromebook_link_defconfig >index dbfbb97..baa0ed8 100644 >--- a/configs/chromebook_link_defconfig >+++ b/configs/chromebook_link_defconfig >@@ -38,5 +38,4 @@ CONFIG_VIDEO_VESA=y > CONFIG_FRAMEBUFFER_SET_VESA_MODE=y > CONFIG_FRAMEBUFFER_VESA_MODE_11A=y > CONFIG_USE_PRIVATE_LIBGCC=y >-CONFIG_SYS_VSNPRINTF=y > CONFIG_TPM=y >diff --git a/configs/chromebox_panther_defconfig >b/configs/chromebox_panther_defconfig >index ed4428f..c368cc0 100644 >--- a/configs/chromebox_panther_defconfig >+++ b/configs/chromebox_panther_defconfig >@@ -33,5 +33,4 @@ CONFIG_VIDEO_VESA=y > CONFIG_FRAMEBUFFER_SET_VESA_MODE=y > CONFIG_FRAMEBUFFER_VESA_MODE_11A=y > CONFIG_USE_PRIVATE_LIBGCC=y >-CONFIG_SYS_VSNPRINTF=y > CONFIG_TPM=y >diff --git a/configs/coreboot-x86_defconfig b/configs/coreboot-x86_defconfig >index cd2be18..fda0db2 100644 >--- a/configs/coreboot-x86_defconfig >+++ b/configs/coreboot-x86_defconfig >@@ -25,5 +25,4 @@ CONFIG_TPM_TIS_LPC=y > CONFIG_USB=y > CONFIG_DM_USB=y > CONFIG_USE_PRIVATE_LIBGCC=y >-CONFIG_SYS_VSNPRINTF=y > CONFIG_TPM=y >diff --git a/configs/crownbay_defconfig b/configs/crownbay_defconfig >index 932d9ec..6bc4b8d 100644 >--- a/configs/crownbay_defconfig >+++ b/configs/crownbay_defconfig >@@ -36,4 +36,3 @@ CONFIG_DM_USB=y > CONFIG_VIDEO_VESA=y > CONFIG_FRAMEBUFFER_SET_VESA_MODE=y > CONFIG_USE_PRIVATE_LIBGCC=y >-CONFIG_SYS_VSNPRINTF=y >diff --git a/configs/galileo_defconfig b/configs/galileo_defconfig >index 0604aa7..925d3ee 100644 >--- a/configs/galileo_defconfig >+++ b/configs/galileo_defconfig >@@ -28,4 +28,3 @@ CONFIG_TIMER=y > CONFIG_USB=y > CONFIG_DM_USB=y > CONFIG_USE_PRIVATE_LIBGCC=y >-CONFIG_SYS_VSNPRINTF=y >diff --git a/configs/minnowmax_defconfig b/configs/minnowmax_defconfig >index 864fd1b..af6a8ec 100644 >--- a/configs/minnowmax_defconfig >+++ b/configs/minnowmax_defconfig >@@ -39,4 +39,3 @@ CONFIG_VIDEO_VESA=y > CONFIG_FRAMEBUFFER_SET_VESA_MODE=y > CONFIG_FRAMEBUFFER_VESA_MODE_11A=y > CONFIG_USE_PRIVATE_LIBGCC=y >-CONFIG_SYS_VSNPRINTF=y >diff --git a/configs/qemu-x86_defconfig b/configs/qemu-x86_defconfig >index 8c86931..b0c935c 100644 >--- a/configs/qemu-x86_defconfig >+++ b/configs/qemu-x86_defconfig >@@ -30,4 +30,3 @@ CONFIG_VIDEO_VESA=y > CONFIG_FRAMEBUFFER_SET_VESA_MODE=y > CONFIG_FRAMEBUFFER_VESA_MODE_111=y > CONFIG_USE_PRIVATE_LIBGCC=y >-CONFIG_SYS_VSNPRINTF=y >diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig >index 731fc25..caa7336 100644 >--- a/configs/sandbox_defconfig >+++ b/configs/sandbox_defconfig >@@ -76,7 +76,6 @@ CONFIG_USB_EMUL=y > CONFIG_USB_STORAGE=y > CONFIG_USB_KEYBOARD=y > CONFIG_SYS_USB_EVENT_POLL=y >-CONFIG_SYS_VSNPRINTF=y > CONFIG_CMD_DHRYSTONE=y > CONFIG_TPM=y > CONFIG_LZ4=y >diff --git a/include/vsprintf.h b/include/vsprintf.h >index b5bc9c1..376f5dd 100644 >--- a/include/vsprintf.h >+++ b/include/vsprintf.h >@@ -124,7 +124,6 @@ int sprintf(char *buf, const char *fmt, ...) > int vsprintf(char *buf, const char *fmt, va_list args); > char *simple_itoa(ulong i); > >-#ifdef CONFIG_SYS_VSNPRINTF > /** > * Format a string and place it in a buffer > * >@@ -199,17 +198,6 @@ int vsnprintf(char *buf, size_t size, const char *fmt, >va_list args); > * See the vsprintf() documentation for format string extensions over C99. > */ > int vscnprintf(char *buf, size_t size, const char *fmt, va_list args); >-#else >-/* >- * Use macros to silently drop the size parameter. Note that the 'cn' >- * versions are the same as the 'n' versions since the functions assume >- * there is always enough buffer space when !CONFIG_SYS_VSNPRINTF >- */ >-#define snprintf(buf, size, fmt, args...) sprintf(buf, fmt, ##args) >-#define scnprintf(buf, size, fmt, args...) sprintf(buf, fmt, ##args) >-#define vsnprintf(buf, size, fmt, args...) vsprintf(buf, fmt, ##args) >-#define vscnprintf(buf, size, fmt, args...) vsprintf(buf, fmt, ##args) >-#endif /* CONFIG_SYS_VSNPRINTF */ > > /** > * print_grouped_ull() - print a value with digits grouped by ',' >diff --git a/lib/Kconfig b/lib/Kconfig >index 9d580e4..46d7034 100644 >--- a/lib/Kconfig >+++ b/lib/Kconfig >@@ -27,15 +27,6 @@ config SYS_HZ > get_timer() must operate in milliseconds and this option must be > set to 1000. > >-config SYS_VSNPRINTF >- bool "Enable safe version of sprintf()" >- help >- Since sprintf() can overflow its buffer, it is common to use >- snprintf() instead, which knows the buffer size and can avoid >- overflow. However, this does increase code size slightly (for >- Thumb-2, about 420 bytes). Enable this option for safety when >- using sprintf() with data you do not control. >- > config USE_TINY_PRINTF > bool "Enable tiny printf() version" > help >diff --git a/lib/vsprintf.c b/lib/vsprintf.c >index 24167a1..874a295 100644 >--- a/lib/vsprintf.c >+++ b/lib/vsprintf.c >@@ -141,7 +141,6 @@ static noinline char *put_dec(char *buf, uint64_t num) > #define SMALL 32 /* Must be 32 == 0x20 */ > #define SPECIAL 64 /* 0x */ > >-#ifdef CONFIG_SYS_VSNPRINTF > /* > * Macro to add a new character to our output string, but only if it will > * fit. The macro moves to the next character position in the output string. >@@ -151,9 +150,6 @@ static noinline char *put_dec(char *buf, uint64_t num) > *(str) = (ch); \ > ++str; \ > } while (0) >-#else >-#define ADDCH(str, ch) (*(str)++ = (ch)) >-#endif > > static char *number(char *buf, char *end, u64 num, > int base, int size, int precision, int type) >@@ -441,13 +437,11 @@ static int vsnprintf_internal(char *buf, size_t size, >const char *fmt, > /* 't' added for ptrdiff_t */ > char *end = buf + size; > >-#ifdef CONFIG_SYS_VSNPRINTF > /* Make sure end is always >= buf - do we want this in U-Boot? */ > if (end < buf) { > end = ((void *)-1); > size = end - buf; > } >-#endif > str = buf; > > for (; *fmt ; ++fmt) { >@@ -609,21 +603,16 @@ repeat: > flags); > } > >-#ifdef CONFIG_SYS_VSNPRINTF > if (size > 0) { > ADDCH(str, '\0'); > if (str > end) > end[-1] = '\0'; > --str; > } >-#else >- *str = '\0'; >-#endif > /* the trailing null byte doesn't count towards the total */ > return str - buf; > } > >-#ifdef CONFIG_SYS_VSNPRINTF > int vsnprintf(char *buf, size_t size, const char *fmt, > va_list args) > { >@@ -666,7 +655,6 @@ int scnprintf(char *buf, size_t size, const char *fmt, ...) > > return i; > } >-#endif /* CONFIG_SYS_VSNPRINT */ > > /** > * Format a string and place it in a buffer (va_list version) >-- >2.6.4 > >_______________________________________________ >U-Boot mailing list >U-Boot@lists.denx.de >http://lists.denx.de/mailman/listinfo/u-boot _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot