From 219905c6bcecfda2ae9724a44653831ccb6f6271 Mon Sep 17 00:00:00 2001 From: Greg King Date: Thu, 9 Jul 2015 10:28:38 -0400 Subject: [PATCH 1/5] Fix two string output functions' handling of their buffer-size parameter. That parameter's type is unsigned; but, the functions return an int. If the size is too big for a signed integer, then return an error code. If the size is zero, then don't write anything into a buffer (the buffer pointer may be NULL). But, do format and count the arguments. --- libsrc/common/vsnprintf.s | 47 +++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/libsrc/common/vsnprintf.s b/libsrc/common/vsnprintf.s index a8ed50e06..94ad072ca 100644 --- a/libsrc/common/vsnprintf.s +++ b/libsrc/common/vsnprintf.s @@ -1,7 +1,8 @@ ; ; int __fastcall__ vsnprintf (char* Buf, size_t size, const char* Format, va_list ap); ; -; Ullrich von Bassewitz, 2009-09-26 +; 2009-09-26, Ullrich von Bassewitz +; 2015-07-09, Greg King ; .export _vsnprintf, vsnprintf @@ -9,6 +10,8 @@ .import _memcpy, __printf .importzp sp, ptr1 + .include "errno.inc" + .macpack generic .data @@ -46,8 +49,10 @@ vsnprintf: sta ccount+1 ; Clear ccount ; Get the size parameter and replace it by a pointer to outdesc. This is to -; build a stack frame for the call to _printf. -; If size is zero, there's nothing to do. +; build a stack frame for the call to _printf. The size must not be greater +; than INT_MAX because the return type is int. If the size is zero, +; then nothing will be written into the buffer; but, the arguments still will +; be formatted and counted. ldy #2 lda (sp),y @@ -58,15 +63,13 @@ vsnprintf: iny lda (sp),y + bmi L9 ; More than $7FFF sta ptr1+1 - ora ptr1 - beq L9 - lda #>outdesc sta (sp),y -; Write size-1 to outdesc.uns +; Write size-1 to outdesc.uns. It will be -1 if there is no buffer. ldy ptr1+1 ldx ptr1 @@ -90,17 +93,18 @@ L1: dex pla jsr __printf -; Terminate the string. The last char is either at bufptr+ccount or -; bufptr+bufsize, whichever is smaller. +; Terminate the string if there is a buffer. The last char. is at either +; bufptr+bufsize or bufptr+ccount, whichever is smaller. + ldx bufsize+1 + bmi L4 ; -1 -- No buffer + lda bufsize+0 + cpx ccount+1 + bne L2 + cmp ccount+0 +L2: bcc L3 lda ccount+0 ldx ccount+1 - cpx bufsize+1 - bne L2 - cmp bufsize+0 -L2: bcc L3 - lda bufsize+0 - ldx bufsize+1 clc L3: adc bufptr+0 sta ptr1 @@ -114,16 +118,14 @@ L3: adc bufptr+0 ; Return the number of bytes written and drop buf - lda ccount+0 +L4: lda ccount+0 ldx ccount+1 jmp incsp2 -; Bail out if size is zero. +; Bail out if size is too high. -L9: pla - pla ; Discard ap - lda #0 - tax +L9: lda #ERANGE + jsr __directerrno ; Return -1 jmp incsp6 ; Drop parameters @@ -146,10 +148,11 @@ out: sbc ccount+0 ; Low byte of bytes already written sta ptr1 lda bufsize+1 + bmi @L9 ; -1 -- No buffer sbc ccount+1 sta ptr1+1 bcs @L0 ; Branch if space left - lda #$00 +@L9: lda #$0000 sta ptr1 sta ptr1+1 ; No space left From 146daa1d0a43883bfa22ddfbb571b3d649c6465e Mon Sep 17 00:00:00 2001 From: Greg King Date: Thu, 9 Jul 2015 14:46:28 -0400 Subject: [PATCH 2/5] Made some string output functions reject an invalid NULL buffer pointer. --- libsrc/common/vsnprintf.s | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/libsrc/common/vsnprintf.s b/libsrc/common/vsnprintf.s index 94ad072ca..01bcd6406 100644 --- a/libsrc/common/vsnprintf.s +++ b/libsrc/common/vsnprintf.s @@ -86,9 +86,16 @@ L1: dex sta bufptr+0 stx bufptr+1 +; There must be a buffer if its size is non-zero. + + bit bufsize+1 + bmi L5 + ora bufptr+1 + bze L0 ; The pointer shouldn't be NULL + ; Restore ap and call _printf - pla +L5: pla tax pla jsr __printf @@ -125,6 +132,11 @@ L4: lda ccount+0 ; Bail out if size is too high. L9: lda #ERANGE + .byte $2C ;(bit $xxxx) + +; NULL buffer pointers usually are invalid. + +L0: lda #EINVAL jsr __directerrno ; Return -1 jmp incsp6 ; Drop parameters From dd7e55820cfcdfa33ae113412d899df8ba0727aa Mon Sep 17 00:00:00 2001 From: Greg King Date: Fri, 17 Jul 2015 20:33:17 -0400 Subject: [PATCH 3/5] Added a test program for the special features of snprintf(). --- testcode/lib/snprintf-test.c | 144 +++++++++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+) create mode 100644 testcode/lib/snprintf-test.c diff --git a/testcode/lib/snprintf-test.c b/testcode/lib/snprintf-test.c new file mode 100644 index 000000000..d3af47d78 --- /dev/null +++ b/testcode/lib/snprintf-test.c @@ -0,0 +1,144 @@ +/* +** Test a function that formats and writes characters into a string buffer. +** This program does not test formatting. It tests some behaviors that are +** specific to the buffer. It tests that certain conditions are handled +** properly. +** +** 2015-07-17, Greg King +*/ + +#include +#include +#include +#include + +static const char format[] = "1234567890\nabcdefghijklmnopqrstuvwxyz\n%u\n%s\n\n"; +#define FORMAT_SIZE (sizeof format - 2u - 2u - 1u) + +#define arg1 12345u +#define ARG1_SIZE (5u) + +static const char arg2[] = "!@#$%^&*()-+"; +#define ARG2_SIZE (sizeof arg2 - 1u) + +#define STRING_SIZE (FORMAT_SIZE + ARG1_SIZE + ARG2_SIZE) + +static char buf[256]; +static int size; + + +static void fillbuf(void) +{ + memset(buf, 0xFF, sizeof buf - 1u); + buf[sizeof buf - 1u] = '\0'; +} + + +unsigned char main(void) +{ + static unsigned char failures = 0; + + /* Show what sprintf() should create. */ + + if ((size = printf(format, arg1, arg2)) != STRING_SIZE) { + ++failures; + printf("printf() gave the wrong size: %d.\n", size); + } + + /* Test the normal behavior of sprintf(). */ + + fillbuf(); + size = sprintf(buf, format, arg1, arg2); + fputs(buf, stdout); + if (size != STRING_SIZE) { + ++failures; + printf("sprintf() gave the wrong size: %d.\n", size); + } + + /* Test the normal behavior of snprintf(). */ + + fillbuf(); + size = snprintf(buf, sizeof buf, format, arg1, arg2); + fputs(buf, stdout); + if (size != STRING_SIZE) { + ++failures; + printf("snprintf(sizeof buf) gave the wrong size:\n %d.\n", size); + } + + /* Does snprintf() return the full-formatted size even when the buffer + ** is short? Does it write beyond the end of that buffer? + */ + + fillbuf(); + size = snprintf(buf, STRING_SIZE - 5u, format, arg1, arg2); + if (size != STRING_SIZE) { + ++failures; + printf("snprintf(STRING_SIZE-5) gave the wrong size:\n %d.\n", size); + } + if (buf[STRING_SIZE - 5u - 1u] != '\0' || buf[STRING_SIZE - 5u] != 0xFF) { + ++failures; + printf("snprintf(STRING_SIZE-5) wrote beyond\n the end of the buffer.\n"); + } + + /* Does snprintf() detect a buffer size that is too big? */ + + fillbuf(); + errno = 0; + size = snprintf(buf, 0x8000, format, arg1, arg2); + if (size >= 0) { + ++failures; + printf("snprintf(0x8000) didn't give an error:\n %d; errno=%d.\n", size, errno); + } else { + printf("snprintf(0x8000) did give an error:\n errno=%d.\n", errno); + } + if (buf[0] != 0xFF) { + ++failures; + printf("snprintf(0x8000) wrote into the buffer.\n"); + } + + /* snprintf() must measure the length of the formatted output even when the + ** buffer size is zero. But, it must not touch the buffer. + */ + + fillbuf(); + size = snprintf(buf, 0, format, arg1, arg2); + if (size != STRING_SIZE) { + ++failures; + printf("snprintf(0) gave the wrong size:\n %d.\n", size); + } + if (buf[0] != 0xFF) { + ++failures; + printf("snprintf(0) wrote into the buffer.\n"); + } + + /* Does sprintf() detect a zero buffer-pointer? */ + + errno = 0; + size = sprintf(NULL, format, arg1, arg2); + if (size >= 0) { + ++failures; + printf("sprintf(NULL) didn't give an error:\n %d; errno=%d.\n", size, errno); + } else { + printf("sprintf(NULL) did give an error:\n errno=%d.\n", errno); + } + + /* snprintf() must measure the length of the formatted output even when the + ** buffer size is zero. A zero pointer is not an error, in that case. + */ + + size = snprintf(NULL, 0, format, arg1, arg2); + if (size != STRING_SIZE) { + ++failures; + printf("snprintf(NULL,0) gave the wrong size:\n %d.\n", size); + } + + if (failures != 0) { + printf("There were %u", failures); + } else { + printf("There were no"); + } + printf(" failures.\nTap a key. "); + cgetc(); + + return failures; +} From 0b6bcb565e7f514c09bcd1c7b44d3e1bc8791e32 Mon Sep 17 00:00:00 2001 From: Greg King Date: Fri, 17 Jul 2015 20:36:56 -0400 Subject: [PATCH 4/5] Fixed a hardware-stack leak. --- libsrc/common/vsnprintf.s | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/libsrc/common/vsnprintf.s b/libsrc/common/vsnprintf.s index 01bcd6406..228e531d0 100644 --- a/libsrc/common/vsnprintf.s +++ b/libsrc/common/vsnprintf.s @@ -2,7 +2,7 @@ ; int __fastcall__ vsnprintf (char* Buf, size_t size, const char* Format, va_list ap); ; ; 2009-09-26, Ullrich von Bassewitz -; 2015-07-09, Greg King +; 2015-07-17, Greg King ; .export _vsnprintf, vsnprintf @@ -131,12 +131,15 @@ L4: lda ccount+0 ; Bail out if size is too high. -L9: lda #ERANGE +L9: ldy #ERANGE .byte $2C ;(bit $xxxx) ; NULL buffer pointers usually are invalid. -L0: lda #EINVAL +L0: ldy #EINVAL + pla ; Drop ap + pla + tya jsr __directerrno ; Return -1 jmp incsp6 ; Drop parameters From a9982de475a04568542683b197539a075dee9384 Mon Sep 17 00:00:00 2001 From: Greg King Date: Sat, 18 Jul 2015 18:23:08 -0400 Subject: [PATCH 5/5] Added _directerrno() to the sim6502/sim65c02 libraries. --- libsrc/sim6502/errno.s | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/libsrc/sim6502/errno.s b/libsrc/sim6502/errno.s index e6c2422c1..d9f7e397e 100644 --- a/libsrc/sim6502/errno.s +++ b/libsrc/sim6502/errno.s @@ -1,11 +1,29 @@ ; -; Oliver Schmidt, 2013-05-16 +; 2013-05-16, Oliver Schmidt +; 2015-07-18, Greg King ; -; extern int errno; +; Helper functions for several high-level functions. ; .include "errno.inc" +; ---------------------------------------------------------------------------- +; int __fastcall__ _directerrno (unsigned char code); +; /* Set errno to a specific error code; and, return -1. Used +; ** by the library. +; */ + +__directerrno: + jsr __seterrno ; Save in errno +fail: lda #$FF ; Return -1 + tax +ok: rts + + +; ---------------------------------------------------------------------------- +; +; extern int _errno; +; .bss __errno: