On Thu, Sep 3, 2015 at 1:21 PM, Joel Sherrill <joel.sherr...@oarcorp.com> wrote: > > > On 9/2/2015 4:54 PM, Martin Galvan wrote: >> >> I also used the 'n' versions of the string functions, #define'd magic >> numbers >> and added a few comments. > > > Great on the 'n' versions! > > One comment. Why is the output buffer now static? I know it > moves it off the stack but what's the consensus on an 80 > byte array on a stack?
There are some reasons: 1) out of the stack, here, means that it will be moved as a static memory. If it doesn't fit, the toolchain will warn. 2) we don't know how much stack memory is left. This is specially important in a debugging function which could be called in a faulty context. 3) 80 bytes is not that low. > > >> >> Updates #2405. >> --- >> cpukit/libmisc/dumpbuf/dumpbuf.c | 121 >> ++++++++++++++++++++++++--------------- >> 1 file changed, 75 insertions(+), 46 deletions(-) >> >> diff --git a/cpukit/libmisc/dumpbuf/dumpbuf.c >> b/cpukit/libmisc/dumpbuf/dumpbuf.c >> index 9d34d42..bb63997 100644 >> --- a/cpukit/libmisc/dumpbuf/dumpbuf.c >> +++ b/cpukit/libmisc/dumpbuf/dumpbuf.c >> @@ -6,7 +6,7 @@ >> */ >> >> /* >> - * COPYRIGHT (c) 1997-2007. >> + * COPYRIGHT (c) 1997-2015. >> * On-Line Applications Research Corporation (OAR). >> * >> * The license and distribution terms for this file may in >> @@ -24,62 +24,91 @@ >> #include <rtems/dumpbuf.h> >> #include <rtems/bspIo.h> >> >> +#define HEX_FMT_LENGTH 3 /* Length of the formatted hex string. */ >> +#define ASCII_FMT_LENGTH 1 /* Length of the formatted ASCII string. */ >> +#define BYTES_PER_ROW 16 /* Amount of bytes from buffer shown in each >> row. */ >> +#define BARS 2 /* Amount of bars in each row. */ >> +/* Max length of each row string. */ >> +#define ROW_LENGTH (BYTES_PER_ROW * (HEX_FMT_LENGTH + ASCII_FMT_LENGTH) + >> BARS) >> + >> /* >> * Put the body below rtems_print_buffer so it won't get inlined. >> */ >> >> -static inline void Dump_Line( >> - const unsigned char *buffer, >> - int length >> -); >> +static void Dump_Line(const unsigned char *buffer, const unsigned int >> length); >> >> -void rtems_print_buffer( >> - const unsigned char *buffer, >> - int length >> -) >> +/** >> + * @brief Print \p length bytes from \p buffer, both in hex and ASCII. >> + * Printing will be done in rows, each showing BYTES_PER_ROW bytes. >> + * @details Non-printable chars will appear as dots. >> + * >> + * @param buffer The buffer we'll print. >> + * @param length Amount of bytes from \p buffer we'll print. This can't >> be >> + * unsigned because we don't have a way to check if we're erroneously >> getting >> + * a negative \p length. >> + */ >> +void rtems_print_buffer(const unsigned char *buffer, const int length) >> { >> + unsigned int i, mod, max; >> >> - int i, mod, max; >> - >> - if ( !length ) return; >> + if (length > 0) { >> + mod = length % BYTES_PER_ROW; >> >> - mod = length % 16; >> + max = length - mod; >> >> - max = length - mod; >> + /* Print length / BYTES_PER_ROW rows. */ >> + for (i = 0; i < max; i += BYTES_PER_ROW) { >> + Dump_Line(&buffer[i], BYTES_PER_ROW); >> + } >> >> - for ( i=0 ; i<max ; i+=16 ) >> - Dump_Line( &buffer[ i ], 16 ); >> - >> - if ( mod ) >> - Dump_Line( &buffer[ max ], mod ); >> + /* Print another row with the remaining bytes. */ >> + if (mod > 0) { >> + Dump_Line(&buffer[max], mod); >> + } >> + } else { >> + printk("Error: length must be greater than zero."); >> + } >> } >> >> -static inline void Dump_Line( >> - const unsigned char *buffer, >> - int length >> -) >> +/** >> + * @brief Print \p length bytes from \p buffer, both in hex and ASCII. >> + * @details Non-printable chars will appear as dots. >> + * >> + * @param buffer The buffer we'll print. >> + * @param length Amount of bytes from \p buffer we'll print. >> + */ >> +static void Dump_Line(const unsigned char *buffer, const unsigned int >> length) >> { >> - >> - int i; >> - char line_buffer[120]; >> - >> - line_buffer[0] = '\0'; >> - >> - for( i=0 ; i<length ; i++ ) >> - sprintf( line_buffer, "%s%02x ", line_buffer, buffer[ i ] ); >> - >> - for( ; i<16 ; i++ ) >> - strcat( line_buffer, " " ); >> - >> - strcat( line_buffer, "|" ); >> - for( i=0 ; i<length ; i++ ) >> - sprintf( line_buffer, "%s%c", line_buffer, >> - isprint( buffer[ i ] ) ? buffer[ i ] : '.' ); >> - >> - for( ; i<16 ; i++ ) >> - strcat( line_buffer, " " ); >> - >> - strcat( line_buffer, "|\n" ); >> - >> - printk( line_buffer ); >> + unsigned int i; >> + static unsigned char line_buffer[ROW_LENGTH] = ""; >> + size_t tmp_len; >> + >> + /* Output the hex value of each byte. */ >> + for (i = 0; i < length; ++i) { >> + snprintf(&line_buffer[i * HEX_FMT_LENGTH], HEX_FMT_LENGTH + 1, >> + "%02x ", buffer[i]); >> + } >> + >> + /* Fill the remaining space with whitespace (if necessary). */ >> + for (; i < BYTES_PER_ROW; ++i) { >> + strncat(line_buffer, " ", HEX_FMT_LENGTH); >> + } >> + >> + /* Append a bar. */ >> + strncat(line_buffer, "|", 1); >> + tmp_len = strnlen(line_buffer, ROW_LENGTH); >> + >> + /* Now output the ASCII glyphs of printable chars. */ >> + for (i = 0; i < length; ++i) { >> + snprintf(&line_buffer[tmp_len + i], ASCII_FMT_LENGTH + 1, >> + "%c", isprint(buffer[i]) ? buffer[i] : '.'); >> + } >> + >> + /* Fill the remaining space with whitespace (if necessary). */ >> + for(; i < BYTES_PER_ROW; i++) { >> + strncat(line_buffer, " ", ASCII_FMT_LENGTH); >> + } >> + >> + /* Append another bar and print the resulting string. */ >> + printk("%s|\n", line_buffer); >> } >> > > -- > Joel Sherrill, Ph.D. Director of Research & Development > joel.sherr...@oarcorp.com On-Line Applications Research > Ask me about RTEMS: a free RTOS Huntsville AL 35805 > Support Available (256) 722-9985 > > _______________________________________________ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel -- Daniel F. Gutson Chief Engineering Officer, SPD San Lorenzo 47, 3rd Floor, Office 5 Córdoba, Argentina Phone: +54 351 4217888 / +54 351 4218211 Skype: dgutson LinkedIn: http://ar.linkedin.com/in/danielgutson _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel