From 54549928f4b8e2a91eeed193dcdcea795e33a160 Mon Sep 17 00:00:00 2001 From: Ehsan Kiani Far Date: Wed, 25 Oct 2023 19:39:18 -0400 Subject: [PATCH] Z: Fix atoe_vsnprintf to support null buffer Std vsnprintf accepts null buffer and returns an estimate Our implementation does not support that Updated the atoe_vsnprintf implementation Signed-off-by: ehsan kiani far --- fvtest/porttest/omrstrTest.cpp | 82 ++++++++--- util/a2e/atoe_utils.c | 243 ++++++++++++++++++--------------- 2 files changed, 193 insertions(+), 132 deletions(-) diff --git a/fvtest/porttest/omrstrTest.cpp b/fvtest/porttest/omrstrTest.cpp index ceeed10bffb..3bf409591fc 100644 --- a/fvtest/porttest/omrstrTest.cpp +++ b/fvtest/porttest/omrstrTest.cpp @@ -1466,52 +1466,60 @@ TEST(PortStrTest, str_WinacpToMutf8) #if defined(J9ZOS390) && !defined(OMR_EBCDIC) /** - * Check whether a null/non-null terminated string is properly handled by fstring(). * It calls atoe_vsnprintf() to verify the functionalities. * @param[in] portLibrary - The port library under test * @param[in] precision - flag for precision specifier * @param[in] buffer - pointer to the buffer intended for atoe_vsnprintf() * @param[in] bufferLength - the buffer length * @param[in] expectedOutput - The expected string + * @param[in] expectedCount - The expected response from atoe_vsnprintf() * @param[in] format - The string format * * @return TEST_PASS on success * TEST_FAIL on error */ static int -printToBuffer(struct OMRPortLibrary *portLibrary, BOOLEAN precision, char *buffer, size_t bufferLength, const char *expectedOutput, const char *format, ...) +printToBuffer(struct OMRPortLibrary *portLibrary, BOOLEAN precision, char *buffer, size_t bufferLength, const char *expectedOutput, int expectedCount, const char *format, ...) { OMRPORT_ACCESS_FROM_OMRPORT(portLibrary); + static const char NULLCHARSTRING[] = "[null]"; int stringLength = 0; + int expectedLength = (NULL == expectedOutput) ? 0 : strlen(expectedOutput); + int actualCount = 0; + const char *nonNullExpectedOutput = (NULL == expectedOutput) ? NULLCHARSTRING : expectedOutput; + const char *nonNullBuffer = (NULL == buffer) ? NULLCHARSTRING : buffer; va_list args; va_start(args, format); - stringLength = atoe_vsnprintf(buffer, bufferLength, format, args); + actualCount = atoe_vsnprintf(buffer, bufferLength, format, args); + stringLength = (NULL == buffer) ? 0 : strlen(buffer); va_end(args); - if (stringLength >= 0) { + if (actualCount >= 0) { if (precision) { - portTestEnv->log("\n\tFinish Testing: String in buffer is: \"%s\", length = %d\n", buffer, stringLength); + portTestEnv->log("\n\tFinish Testing: String in buffer is: \"%s\", length = %d\n", nonNullBuffer, stringLength); } else { - portTestEnv->log("\n\tFinish Testing without precision specifier: String in buffer is: \"%s\", length = %d\n", buffer, stringLength); + portTestEnv->log("\n\tFinish Testing without precision specifier: String in buffer is: \"%s\", length = %d\n", nonNullBuffer, stringLength); } - if ((stringLength == strlen(expectedOutput)) - && (0 == memcmp(buffer, expectedOutput, stringLength)) + if ((stringLength == expectedLength) + && (actualCount == expectedCount) + && ((NULL == buffer) || (0 == memcmp(buffer, expectedOutput, stringLength + 1))) ) { portTestEnv->log("\n\tComparing against the expected output: PASSED.\n"); return TEST_PASS; } else { - portTestEnv->log(LEVEL_ERROR, "\n\tComparing against the expected output: FAILED. Expected string: \"%s\", length = %d\n", expectedOutput, strlen(expectedOutput)); + portTestEnv->log(LEVEL_ERROR, "\n\tComparing against the expected output: FAILED. Expected string: \"%s\", length = %d\n", nonNullExpectedOutput, expectedLength); return TEST_FAIL; } } else { - portTestEnv->log(LEVEL_ERROR, "\n\tComparing against the expected output: FAILED. stringLength < 0, Expected string: \"%s\", length = %d\n", expectedOutput, strlen(expectedOutput)); + portTestEnv->log(LEVEL_ERROR, "\n\tComparing against the expected output: FAILED. stringLength < 0, Expected string: \"%s\", length = %d\n", nonNullExpectedOutput, expectedLength); } } /** - * Verify that null/non-null terminated strings are properly handled in fstring(). + * Verify that null/non-null terminated strings are properly handled. + * Also checks if number, long number, long long number, and float are properly handled. * It actually calls invoke atoe_vsnprintf() to run the tests through sub-functions. */ TEST(PortStrTest, str_test_atoe_vsnprintf) @@ -1538,35 +1546,71 @@ TEST(PortStrTest, str_test_atoe_vsnprintf) /* Neither min_width nor precision is specified for a null terminated input string*/ portTestEnv->log("\n\tTesting case 1\n"); - rc |= printToBuffer(OMRPORTLIB, TRUE, buffer0, bufferLength0, expectedOutput1, "%s", nullTerminatedString); + rc |= printToBuffer(OMRPORTLIB, TRUE, buffer0, bufferLength0, expectedOutput1, strlen(expectedOutput1), "%s", nullTerminatedString); /* min_width is less than the length of inputString */ portTestEnv->log("\n\tTesting case 2\n"); - rc |= printToBuffer(OMRPORTLIB, FALSE, buffer2, bufferLength2, expectedOutput2, "%*s", 2, nonNullTerminatedString); + rc |= printToBuffer(OMRPORTLIB, FALSE, buffer2, bufferLength2, expectedOutput2, strlen(nullTerminatedString), "%*s", 2, nullTerminatedString); /* min_width is equal to the length of inputString (buffer length > min_width) */ portTestEnv->log("\n\tTesting case 3\n"); - rc |= printToBuffer(OMRPORTLIB, FALSE, buffer1, bufferLength1, expectedOutput1, "%*s", 4, nullTerminatedString); + rc |= printToBuffer(OMRPORTLIB, FALSE, buffer1, bufferLength1, expectedOutput1, strlen(expectedOutput1), "%*s", 4, nullTerminatedString); /* min_width is greater than the length of inputString (buffer length > min_width) */ portTestEnv->log("\n\tTesting case 4\n"); - rc |= printToBuffer(OMRPORTLIB, FALSE, buffer3, bufferLength3, expectedOutput3, "%*s", 10, nullTerminatedString); + rc |= printToBuffer(OMRPORTLIB, FALSE, buffer3, bufferLength3, expectedOutput3, strlen(expectedOutput3), "%*s", 10, nullTerminatedString); /* precision is equal to the length of inputString */ portTestEnv->log("\n\tTesting case 5\n"); - rc |= printToBuffer(OMRPORTLIB, TRUE, buffer0, bufferLength0, expectedOutput1, "%.*s", 4, nonNullTerminatedString); + rc |= printToBuffer(OMRPORTLIB, TRUE, buffer0, bufferLength0, expectedOutput1, strlen(expectedOutput1), "%.*s", 4, nonNullTerminatedString); /* precision is less than the length of inputString */ portTestEnv->log("\n\tTesting case 6\n"); - rc |= printToBuffer(OMRPORTLIB, TRUE, buffer0, bufferLength0, expectedOutput4, "%.*s", 2, nonNullTerminatedString); + rc |= printToBuffer(OMRPORTLIB, TRUE, buffer0, bufferLength0, expectedOutput4, strlen(expectedOutput4), "%.*s", 2, nonNullTerminatedString); /* both min_width and precision are equal to the length of inputString */ portTestEnv->log("\n\tTesting case 7\n"); - rc |= printToBuffer(OMRPORTLIB, TRUE, buffer0, bufferLength0, expectedOutput1, "%*.*s", 4, 4, nonNullTerminatedString); + rc |= printToBuffer(OMRPORTLIB, TRUE, buffer0, bufferLength0, expectedOutput1, strlen(expectedOutput1), "%*.*s", 4, 4, nonNullTerminatedString); /* the length of inputString is equal to precision but less than min_width */ portTestEnv->log("\n\tTesting case 8\n"); - rc |= printToBuffer(OMRPORTLIB, TRUE, buffer0, bufferLength0, expectedOutput3, "%*.*s", 10, 4, nonNullTerminatedString); + rc |= printToBuffer(OMRPORTLIB, TRUE, buffer0, bufferLength0, expectedOutput3, strlen(expectedOutput3), "%*.*s", 10, 4, nonNullTerminatedString); + + /* the buffer is NULL. expect to estimate the length */ + portTestEnv->log("\n\tTesting case 9\n"); + rc |= printToBuffer(OMRPORTLIB, FALSE, NULL, 0, NULL, strlen(nullTerminatedString), "%s", nullTerminatedString); + + /* number with min_width */ + portTestEnv->log("\n\tTesting case 10\n"); + rc |= printToBuffer(OMRPORTLIB, FALSE, buffer0, bufferLength0, " -123", 6, "%*d", 6, -123); + + /* number with precision */ + portTestEnv->log("\n\tTesting case 11\n"); + rc |= printToBuffer(OMRPORTLIB, TRUE, buffer0, bufferLength0, "-000123", 7, "%.*d", 6, -123); + + /* long number with min_width */ + portTestEnv->log("\n\tTesting case 12\n"); + rc |= printToBuffer(OMRPORTLIB, FALSE, buffer0, bufferLength0, " -1234", 7, "%*ld", 7, -1234L); + + /* long number with precision */ + portTestEnv->log("\n\tTesting case 13\n"); + rc |= printToBuffer(OMRPORTLIB, TRUE, buffer0, bufferLength0, "-0001234", 8, "%.*ld", 7, -1234L); + + /* long long number with min_width */ + portTestEnv->log("\n\tTesting case 14\n"); + rc |= printToBuffer(OMRPORTLIB, FALSE, buffer0, bufferLength0, " -12345", 8, "%*lld", 8, -12345LL); + + /* long long number with precision */ + portTestEnv->log("\n\tTesting case 15\n"); + rc |= printToBuffer(OMRPORTLIB, TRUE, buffer0, bufferLength0, "-00012345", 9, "%.*lld", 8, -12345LL); + + /* float number with no precision */ + portTestEnv->log("\n\tTesting case 16\n"); + rc |= printToBuffer(OMRPORTLIB, FALSE, buffer0, bufferLength0, "-123.456000", 11, "%f", -123.456); + + /* float number with precision */ + portTestEnv->log("\n\tTesting case 17\n"); + rc |= printToBuffer(OMRPORTLIB, TRUE, buffer0, bufferLength0, "-123.46", 7, "%.2f", -123.456); if (TEST_PASS != rc) { outputErrorMessage(PORTTEST_ERROR_ARGS, "\n\tTEST FAILED.\n"); diff --git a/util/a2e/atoe_utils.c b/util/a2e/atoe_utils.c index 8b6a124ef42..1de2ee651e2 100644 --- a/util/a2e/atoe_utils.c +++ b/util/a2e/atoe_utils.c @@ -55,7 +55,7 @@ #define ERROR_RETVAL -1 #define SUCCESS 0 -#define CheckRet(x) { if ((x) == ERROR_RETVAL) return ERROR_RETVAL; } +#define CheckRet(x) { if ((x) < 0) return ERROR_RETVAL; } typedef struct InstanceData { char *buffer; @@ -67,9 +67,8 @@ typedef struct InstanceData { * description - Print a charater to InstanceData buffer * parameters - this Structure holding the receiving buffer * c Character to add to buffer - * returns - int return code, 0 for success *************************************************************************/ -static int +static void pchar(InstanceData *this, int c) { #if 0 @@ -101,11 +100,10 @@ pchar(InstanceData *this, int c) } #endif /* 0 */ - if (this->buffer >= this->end) { - return ERROR_RETVAL; + if (this->buffer < this->end) { + *this->buffer = c; } - *this->buffer++ = c; - return SUCCESS; + *this->buffer++; } /************************************************************************** @@ -115,14 +113,14 @@ pchar(InstanceData *this, int c) * str String to add to buffer * left_justify Left justify string flag * min_width Minimum width of string added to buffer - * precision + * precision Maximum chars to take from the argument * returns - int return code, 0 for success *************************************************************************/ static int -fstring(InstanceData *this, char *str, int left_justify, int min_width, +fstring(InstanceData *this, const char *str, int left_justify, int min_width, int precision) { - int pad_length; + int pad_length = 0; int width = 0; const char *p = str; @@ -130,7 +128,7 @@ fstring(InstanceData *this, char *str, int left_justify, int min_width, return ERROR_RETVAL; } - for (width = 0; width < precision; width++) { + for (width = 0; (width < precision) || (precision < 0); width++) { if ('\0' == str[width]) { break; } @@ -138,25 +136,23 @@ fstring(InstanceData *this, char *str, int left_justify, int min_width, if (width < min_width) { pad_length = min_width - width; - } else { - pad_length = 0; } if (left_justify) { while (pad_length > 0) { - CheckRet(pchar(this, ' ')); + pchar(this, ' '); pad_length -= 1; } } while (width-- > 0) { - CheckRet(pchar(this, *p)); + pchar(this, *p); p += 1; } if (!left_justify) { while (pad_length > 0) { - CheckRet(pchar(this, ' ')); + pchar(this, ' '); pad_length -= 1; } } @@ -176,9 +172,9 @@ typedef enum { * value The value to format * format_type Character flag specifying format type * left_justify Left justify number flag - * min_width Minimum number of charaters value will + * min_width Minimum number of characters value will * occupy - * precision + * precision Fix the width with leading zeros * zero_pad Pad number with zeros, flag * returns - int return code, 0 for success *************************************************************************/ @@ -187,11 +183,12 @@ fnumber(InstanceData *this, long value, int format_type, int left_justify, int min_width, int precision, bool_t zero_pad) { int sign_value = 0; - unsigned long uvalue; + unsigned long uvalue = 0; char convert[MAX_DIGITS + 1]; int place = 0; int pad_length = 0; - static char digits[] = "0123456789abcdef"; + int zero_pad_length = 0; + static const char digits[] = "0123456789abcdef"; int base = 0; bool_t caps = FALSE; bool_t add_sign = FALSE; @@ -242,42 +239,49 @@ fnumber(InstanceData *this, long value, int format_type, int left_justify, } while (uvalue); convert[place] = 0; - pad_length = min_width - place; - if (pad_length < 0) { - pad_length = 0; - } - if (left_justify) { - if (zero_pad && pad_length > 0) { - if (sign_value) { - CheckRet(pchar(this, sign_value)); - --pad_length; - sign_value = 0; - } - while (pad_length > 0) { - CheckRet(pchar(this, '0')); - --pad_length; - } - } else { - while (pad_length > 0) { - CheckRet(pchar(this, ' ')); - --pad_length; - } + if (precision > 0) { + zero_pad_length = precision - place; + } else if (zero_pad && left_justify) { + zero_pad_length = min_width - place; + if (sign_value) { + zero_pad_length--; } } - if (sign_value) { - CheckRet(pchar(this, sign_value)); + + if (zero_pad_length < 0) { + zero_pad_length = 0; } - while (place > 0 && --precision >= 0) { - CheckRet(pchar(this, convert[--place])); + pad_length = min_width - (place + zero_pad_length); + if (sign_value) { + pad_length--; } - if (!left_justify) { + if (left_justify) { while (pad_length > 0) { - CheckRet(pchar(this, ' ')); + pchar(this, ' '); --pad_length; } } + + if (sign_value) { + pchar(this, sign_value); + } + + while (zero_pad_length > 0) { + pchar(this, '0'); + zero_pad_length--; + } + + while (place > 0) { + pchar(this, convert[--place]); + } + + while (pad_length > 0) { + pchar(this, ' '); + --pad_length; + } + return SUCCESS; } @@ -289,9 +293,9 @@ fnumber(InstanceData *this, long value, int format_type, int left_justify, * value Number to convert * format_type Character flag defining format * left_justify Left justify number flag - * min_width Minimum number of charaters value will + * min_width Minimum number of characters value will * occupy - * precision + * precision Fix the width with leading zeros * zero_pad Pad number with zeros, flag * returns - int return code, 0 for success *======================================================================= @@ -301,11 +305,12 @@ flongnumber(InstanceData *this, signed long long value, int format_type, int lef int min_width, int precision, bool_t zero_pad) { int sign_value = 0; - unsigned long long uvalue; + unsigned long long uvalue = 0; char convert[MAX_DIGITS + 1]; int place = 0; int pad_length = 0; - static char digits[] = "0123456789abcdef"; + int zero_pad_length = 0; + static const char digits[] = "0123456789abcdef"; int base = 0; bool_t caps = FALSE; bool_t add_sign = FALSE; @@ -356,42 +361,49 @@ flongnumber(InstanceData *this, signed long long value, int format_type, int lef } while (uvalue); convert[place] = 0; - pad_length = min_width - place; - if (pad_length < 0) { - pad_length = 0; - } - if (left_justify) { - if (zero_pad && pad_length > 0) { - if (sign_value) { - CheckRet(pchar(this, sign_value)); - --pad_length; - sign_value = 0; - } - while (pad_length > 0) { - CheckRet(pchar(this, '0')); - --pad_length; - } - } else { - while (pad_length > 0) { - CheckRet(pchar(this, ' ')); - --pad_length; - } + if (precision > 0) { + zero_pad_length = precision - place; + } else if (zero_pad && left_justify) { + zero_pad_length = min_width - place; + if (sign_value) { + zero_pad_length--; } } - if (sign_value) { - CheckRet(pchar(this, sign_value)); + + if (zero_pad_length < 0) { + zero_pad_length = 0; } - while (place > 0 && --precision >= 0) { - CheckRet(pchar(this, convert[--place])); + pad_length = min_width - (place + zero_pad_length); + if (sign_value) { + pad_length--; } - if (!left_justify) { + if (left_justify) { while (pad_length > 0) { - CheckRet(pchar(this, ' ')); + pchar(this, ' '); --pad_length; } } + + if (sign_value) { + pchar(this, sign_value); + } + + while (zero_pad_length > 0) { + pchar(this, '0'); + zero_pad_length--; + } + + while (place > 0) { + pchar(this, convert[--place]); + } + + while (pad_length > 0) { + pchar(this, ' '); + --pad_length; + } + return SUCCESS; } @@ -409,27 +421,25 @@ flongnumber(InstanceData *this, signed long long value, int format_type, int lef int atoe_vsnprintf(char *str, size_t count, const char *fmt, va_list args) { - char *strvalue; - const char *pattern; /*ibm@8665*/ - long value; + char *strvalue = NULL; + const char *pattern = NULL; /*ibm@8665*/ + long value = 0; InstanceData this; - bool_t left_justify, zero_pad; - bool_t long_flag, long_long_flag; /*ibm@9094*/ - bool_t fPrecision; - int min_width, precision, ch; - static char NULLCHARSTRING[] = "[null]"; /*ibm@029013*/ + bool_t left_justify = FALSE; + bool_t zero_pad = FALSE; + bool_t long_flag = FALSE; + bool_t long_long_flag = FALSE; /*ibm@9094*/ + bool_t fPrecision = FALSE; + int min_width = 0; + int precision = 0; + int ch = 0; + static const char NULLCHARSTRING[] = "[null]"; /*ibm@029013*/ /*ibm@029013*/ if (fmt == NULL) { /*ibm@029013*/ fmt = NULLCHARSTRING; /*ibm@029013*/ } /*ibm@029013*/ - if (str == NULL) { - return ERROR_RETVAL; - } - str[0] = '\0'; - this.buffer = str; - this.end = str + count - 1; - *this.end = '\0'; /* ensure null-termination in case of failure */ + this.end = this.buffer + count; while ((ch = *fmt++) != 0) { if (ch == '%') { @@ -440,7 +450,7 @@ atoe_vsnprintf(char *str, size_t count, const char *fmt, va_list args) pattern = fmt - 1; /*ibm@8665*/ left_justify = TRUE; min_width = 0; - precision = this.end - this.buffer; + precision = -1; /* -1 means unspecified */ next_char: ch = *fmt++; @@ -450,7 +460,7 @@ atoe_vsnprintf(char *str, size_t count, const char *fmt, va_list args) case '-': left_justify = FALSE; goto next_char; - case '+': /*ibm@8665*/ + case '+': /*ibm@8665*/ left_justify = TRUE; /*ibm@8665*/ goto next_char; /*ibm@8665*/ case '0': /* set zero padding if min_width not set */ @@ -467,7 +477,7 @@ atoe_vsnprintf(char *str, size_t count, const char *fmt, va_list args) case '7': case '8': case '9': - if (fPrecision == TRUE) { + if (fPrecision) { precision = precision * 10 + (ch - '0'); } else { min_width = min_width * 10 + (ch - '0'); @@ -479,7 +489,7 @@ atoe_vsnprintf(char *str, size_t count, const char *fmt, va_list args) goto next_char; case '*': { int temp_precision = va_arg(args, int); - if (fPrecision == TRUE) { + if (fPrecision) { precision = temp_precision; } else { min_width = temp_precision; @@ -501,12 +511,12 @@ atoe_vsnprintf(char *str, size_t count, const char *fmt, va_list args) break; case 'c': ch = va_arg(args, int); - CheckRet(pchar(&this, ch)); + pchar(&this, ch); break; case '%': - CheckRet(pchar(&this, '%')); + pchar(&this, '%'); #if 0 /* J9 CMVC defect 74726 */ - CheckRet(pchar(&this, '%')); /*ibm@5203 */ + pchar(&this, '%'); /*ibm@5203 */ #endif break; case 'd': @@ -540,27 +550,28 @@ atoe_vsnprintf(char *str, size_t count, const char *fmt, va_list args) case 'f': case 'F': case 'g': - case 'G': - { + case 'G': { /* ibm@8665 * Add floating point support for dbl2str & flt2str */ - char *b; - int len; - - b = a2e((char *)pattern, fmt - pattern); + char *tempPattern = a2e((char *)pattern, fmt - pattern); + size_t freeCap = (this.end > this.buffer) ? (this.end - this.buffer) : 0; /* Extract a double from args, this works for both doubles * and floats, * NB if we use float for a single precision floating * point number the result is wrong. */ - len = sprintf(this.buffer, b, va_arg(args, double)); - free(b); - b = e2a_string(this.buffer); - strcpy(this.buffer, b); - free(b); - this.buffer += len; + double argument = va_arg(args, double); + int requiredSpace = snprintf(this.buffer, freeCap, tempPattern, argument); + free(tempPattern); + CheckRet(requiredSpace); + if ((freeCap > 0) && (requiredSpace > 0)) { + char *tempAsciiValue = e2a_string(this.buffer); + strcpy(this.buffer, tempAsciiValue); + free(tempAsciiValue); + } + this.buffer += requiredSpace; } break; default: @@ -581,11 +592,17 @@ atoe_vsnprintf(char *str, size_t count, const char *fmt, va_list args) return ERROR_RETVAL; } } else { - CheckRet(pchar(&this, ch)); + pchar(&this, ch); + } + } + if (count > 0) { + if (this.buffer < this.end) { + *this.buffer = '\0'; + } else { + this.end[-1] = '\0'; } } - *this.buffer = '\0'; - return strlen(str); + return this.buffer - str; } /* END OF FILE */