[PATCH v2 4/5] cpukit/libmisc/dumpbuf/dumpbuf.c: Fix undefined behavior for sprintf()
Daniel Gutson
daniel.gutson at tallertechnologies.com
Thu Sep 3 17:18:23 UTC 2015
On Thu, Sep 3, 2015 at 1:21 PM, Joel Sherrill <joel.sherrill at 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.sherrill at 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 at 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
More information about the devel
mailing list