Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Misra compliance #79

Closed
wants to merge 13 commits into from
Closed

Misra compliance #79

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
740b878
Rule 13.3 increment/decrement with other side effects
Dec 24, 2021
7398ba0
Merge collapsible if statements (not strictly MISRA)
Dec 24, 2021
e0aff68
Unmodified pointers should be const (MISRA C:2004, 16.7 )
Dec 26, 2021
62bcd9d
Removed redundant cast
Dec 26, 2021
f5c7327
MISRA C:2012, 20.1 - #include directives should only be preceded by p…
Dec 26, 2021
c43886f
Revert "Unmodified pointers should be const (MISRA C:2004, 16.7 )"
Dec 27, 2021
950c40e
MISRA C:2004, 16.9 - A function identifier shall only be used with ei…
Dec 27, 2021
7d74780
MISRA C++:2008, 18-0-1 - The C library shall not be used.
Dec 30, 2021
bee6c0d
MISRA C:2004, 19.10
Jan 2, 2022
a6cb311
MISRA C:2004, 6.3 - typedefs that indicate size and signedness should…
Jan 2, 2022
1dfd730
Change to size_t for internal strnlen return value
Jan 3, 2022
cdeac63
Split function declarations into standard and custom
Jan 5, 2022
a993682
Merge branch 'develop' into MISRA-compliance
sanderh255 Jan 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 38 additions & 39 deletions src/printf/printf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,6 @@ static inline int get_exp2(double_with_bit_access x)
// output function type
typedef void (*out_fct_type)(char character, void* buffer, size_t idx, size_t maxlen);


// wrapper (used as buffer) for output function type
typedef struct {
void (*fct)(char character, void* arg);
Expand Down Expand Up @@ -261,11 +260,11 @@ static inline void out_wrapped_function(char character, void* wrapped_function,

// internal secure strlen
// @return The length of the string (excluding the terminating 0) limited by 'maxsize'
static inline unsigned int strnlen_s_(const char* str, size_t maxsize)
static inline printf_uint_t strnlen_s_(const char* str, size_t maxsize)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I could just make it size_t?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion, done in an additional commit.

{
const char* s;
for (s = str; *s && maxsize--; ++s);
return (unsigned int)(s - str);
return (printf_uint_t)(s - str);
}


Expand All @@ -278,18 +277,18 @@ static inline bool is_digit_(char ch)


// internal ASCII string to unsigned int conversion
static unsigned int atoi_(const char** str)
static printf_uint_t atoi_(const char** str)
{
unsigned int i = 0U;
printf_uint_t i = 0U;
while (is_digit_(**str)) {
i = i * 10U + (unsigned int)(*((*str)++) - '0');
i = i * 10U + (printf_uint_t)(*((*str)++) - '0');
}
return i;
}


// output the specified string in reverse, taking care of any zero-padding
static size_t out_rev_(out_fct_type out, char* buffer, size_t idx, size_t maxlen, const char* buf, size_t len, unsigned int width, unsigned int flags)
static size_t out_rev_(out_fct_type out, char* buffer, size_t idx, size_t maxlen, const char* buf, size_t len, printf_uint_t width, printf_uint_t flags)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose I could have a distinct type for the flag holder integer.

{
const size_t start_idx = idx;

Expand Down Expand Up @@ -318,7 +317,7 @@ static size_t out_rev_(out_fct_type out, char* buffer, size_t idx, size_t maxlen

// Invoked by print_integer after the actual number has been printed, performing necessary
// work on the number's prefix (as the number is initially printed in reverse order)
static size_t print_integer_finalization(out_fct_type out, char* buffer, size_t idx, size_t maxlen, char* buf, size_t len, bool negative, numeric_base_t base, unsigned int precision, unsigned int width, unsigned int flags)
static size_t print_integer_finalization(out_fct_type out, char* buffer, size_t idx, size_t maxlen, char* buf, size_t len, bool negative, numeric_base_t base, printf_uint_t precision, printf_uint_t width, printf_uint_t flags)
{
size_t unpadded_len = len;

Expand Down Expand Up @@ -385,7 +384,7 @@ static size_t print_integer_finalization(out_fct_type out, char* buffer, size_t
}

// An internal itoa-like function
static size_t print_integer(out_fct_type out, char* buffer, size_t idx, size_t maxlen, printf_unsigned_value_t value, bool negative, numeric_base_t base, unsigned int precision, unsigned int width, unsigned int flags)
static size_t print_integer(out_fct_type out, char* buffer, size_t idx, size_t maxlen, printf_unsigned_value_t value, bool negative, numeric_base_t base, printf_uint_t precision, printf_uint_t width, printf_uint_t flags)
{
char buf[PRINTF_INTEGER_BUFFER_SIZE];
size_t len = 0U;
Expand Down Expand Up @@ -436,7 +435,7 @@ static const double powers_of_10[NUM_DECIMAL_DIGITS_IN_INT64_T] = {
// Break up a double number - which is known to be a finite non-negative number -
// into its base-10 parts: integral - before the decimal point, and fractional - after it.
// Taken the precision into account, but does not change it even internally.
static struct double_components get_components(double number, unsigned int precision)
static struct double_components get_components(double number, printf_uint_t precision)
{
struct double_components number_;
number_.is_negative = get_sign(number);
Expand Down Expand Up @@ -511,7 +510,7 @@ struct scaling_factor update_normalization(struct scaling_factor sf, double extr
}

#if PRINTF_SUPPORT_EXPONENTIAL_SPECIFIERS
static struct double_components get_normalized_components(bool negative, unsigned int precision, double non_normalized, struct scaling_factor normalization)
static struct double_components get_normalized_components(bool negative, printf_uint_t precision, double non_normalized, struct scaling_factor normalization)
{
struct double_components components;
components.is_negative = negative;
Expand Down Expand Up @@ -551,13 +550,13 @@ static struct double_components get_normalized_components(bool negative, unsigne
#endif

static size_t print_broken_up_decimal(
struct double_components number_, out_fct_type out, char *buffer, size_t idx, size_t maxlen, unsigned int precision,
unsigned int width, unsigned int flags, char *buf, size_t len)
struct double_components number_, out_fct_type out, char *buffer, size_t idx, size_t maxlen, printf_uint_t precision,
printf_uint_t width, printf_uint_t flags, char *buf, size_t len)
{
if (precision != 0U) {
// do fractional part, as an unsigned number

unsigned int count = precision;
printf_uint_t count = precision;

// %g/%G mandates we skip the trailing 0 digits...
if ((flags & FLAGS_ADAPT_EXP) && !(flags & FLAGS_HASH) && (number_.fractional > 0)) {
Expand Down Expand Up @@ -633,15 +632,15 @@ static size_t print_broken_up_decimal(
}

// internal ftoa for fixed decimal floating point
static size_t print_decimal_number(out_fct_type out, char* buffer, size_t idx, size_t maxlen, double number, unsigned int precision, unsigned int width, unsigned int flags, char* buf, size_t len)
static size_t print_decimal_number(out_fct_type out, char* buffer, size_t idx, size_t maxlen, double number, printf_uint_t precision, printf_uint_t width, printf_uint_t flags, char* buf, size_t len)
{
struct double_components value_ = get_components(number, precision);
return print_broken_up_decimal(value_, out, buffer, idx, maxlen, precision, width, flags, buf, len);
}

#if PRINTF_SUPPORT_EXPONENTIAL_SPECIFIERS
// internal ftoa variant for exponential floating-point type, contributed by Martijn Jasperse <m.jasperse@gmail.com>
static size_t print_exponential_number(out_fct_type out, char* buffer, size_t idx, size_t maxlen, double number, unsigned int precision, unsigned int width, unsigned int flags, char* buf, size_t len)
static size_t print_exponential_number(out_fct_type out, char* buffer, size_t idx, size_t maxlen, double number, printf_uint_t precision, printf_uint_t width, printf_uint_t flags, char* buf, size_t len)
{
const bool negative = get_sign(number);
// This number will decrease gradually (by factors of 10) as we "extract" the exponent out of it
Expand Down Expand Up @@ -732,9 +731,9 @@ static size_t print_exponential_number(out_fct_type out, char* buffer, size_t id

// the exp10 format is "E%+03d" and largest possible exp10 value for a 64-bit double
// is "307" (for 2^1023), so we set aside 4-5 characters overall
unsigned int exp10_part_width = fall_back_to_decimal_only_mode ? 0U : (PRINTF_ABS(exp10) < 100) ? 4U : 5U;
printf_uint_t exp10_part_width = fall_back_to_decimal_only_mode ? 0U : (PRINTF_ABS(exp10) < 100) ? 4U : 5U;

unsigned int decimal_part_width =
printf_uint_t decimal_part_width =
((flags & FLAGS_LEFT) && exp10_part_width) ?
// We're padding on the right, so the width constraint is the exponent part's
// problem, not the decimal part's, so we'll use as many characters as we need:
Expand Down Expand Up @@ -768,7 +767,7 @@ static size_t print_exponential_number(out_fct_type out, char* buffer, size_t id
#endif // PRINTF_SUPPORT_EXPONENTIAL_SPECIFIERS


static size_t print_floating_point(out_fct_type out, char* buffer, size_t idx, size_t maxlen, double value, unsigned int precision, unsigned int width, unsigned int flags, bool prefer_exponential)
static size_t print_floating_point(out_fct_type out, char* buffer, size_t idx, size_t maxlen, double value, printf_uint_t precision, printf_uint_t width, printf_uint_t flags, bool prefer_exponential)
{
char buf[PRINTF_FTOA_BUFFER_SIZE];
size_t len = 0U;
Expand Down Expand Up @@ -814,9 +813,9 @@ static size_t print_floating_point(out_fct_type out, char* buffer, size_t idx, s
#endif // (PRINTF_SUPPORT_DECIMAL_SPECIFIERS || PRINTF_SUPPORT_EXPONENTIAL_SPECIFIERS)

// internal vsnprintf
static int _vsnprintf(out_fct_type out, char* buffer, const size_t maxlen, const char* format, va_list va)
static printf_int_t _vsnprintf(out_fct_type out, char* buffer, const size_t maxlen, const char* format, va_list va)
{
unsigned int flags, width, precision, n;
printf_uint_t flags, width, precision, n;
size_t idx = 0U;

if (!buffer) {
Expand Down Expand Up @@ -860,10 +859,10 @@ static int _vsnprintf(out_fct_type out, char* buffer, const size_t maxlen, const
const int w = va_arg(va, int);
if (w < 0) {
flags |= FLAGS_LEFT; // reverse padding
width = (unsigned int)-w;
width = (printf_uint_t)-w;
}
else {
width = (unsigned int)w;
width = (printf_uint_t)w;
}
format++;
}
Expand All @@ -878,7 +877,7 @@ static int _vsnprintf(out_fct_type out, char* buffer, const size_t maxlen, const
}
else if (*format == '*') {
const int precision_ = va_arg(va, int);
precision = precision_ > 0 ? (unsigned int)precision_ : 0U;
precision = precision_ > 0 ? (printf_uint_t)precision_ : 0U;
format++;
}
}
Expand Down Expand Up @@ -985,7 +984,7 @@ static int _vsnprintf(out_fct_type out, char* buffer, const size_t maxlen, const
idx = print_integer(out, buffer, idx, maxlen, (printf_unsigned_value_t) va_arg(va, unsigned long), false, base, precision, width, flags);
}
else {
const unsigned int value = (flags & FLAGS_CHAR) ? (unsigned char)va_arg(va, unsigned int) : (flags & FLAGS_SHORT) ? (unsigned short int)va_arg(va, unsigned int) : va_arg(va, unsigned int);
const printf_uint_t value = (flags & FLAGS_CHAR) ? (unsigned char)va_arg(va, printf_uint_t) : (flags & FLAGS_SHORT) ? (unsigned short int)va_arg(va, printf_uint_t) : va_arg(va, printf_uint_t);
idx = print_integer(out, buffer, idx, maxlen, (printf_unsigned_value_t) value, false, base, precision, width, flags);
}
}
Expand All @@ -1012,7 +1011,7 @@ static int _vsnprintf(out_fct_type out, char* buffer, const size_t maxlen, const
break;
#endif // PRINTF_SUPPORT_EXPONENTIAL_SPECIFIERS
case 'c' : {
unsigned int l = 1U;
printf_uint_t l = 1U;
// pre padding
if (!(flags & FLAGS_LEFT)) {
while (l++ < width) {
Expand All @@ -1037,7 +1036,7 @@ static int _vsnprintf(out_fct_type out, char* buffer, const size_t maxlen, const
idx = out_rev_(out, buffer, idx, maxlen, ")llun(", 6, width, flags);
}
else {
unsigned int l = strnlen_s_(p, precision ? precision : (size_t)-1);
printf_uint_t l = strnlen_s_(p, precision ? precision : (size_t)-1);
// pre padding
if (flags & FLAGS_PRECISION) {
l = (l < precision ? l : precision);
Expand Down Expand Up @@ -1113,64 +1112,64 @@ static int _vsnprintf(out_fct_type out, char* buffer, const size_t maxlen, const

///////////////////////////////////////////////////////////////////////////////

int printf_(const char* format, ...)
printf_int_t printf_(const char* format, ...)
{
va_list va;
va_start(va, format);
char buffer[1];
const int ret = _vsnprintf(&out_putchar, buffer, (size_t)-1, format, va);
const printf_int_t ret = _vsnprintf(&out_putchar, buffer, (size_t)-1, format, va);
va_end(va);
return ret;
}


int sprintf_(char* buffer, const char* format, ...)
printf_int_t sprintf_(char* buffer, const char* format, ...)
{
va_list va;
va_start(va, format);
const int ret = _vsnprintf(out_buffer, buffer, (size_t)-1, format, va);
const printf_int_t ret = _vsnprintf(out_buffer, buffer, (size_t)-1, format, va);
va_end(va);
return ret;
}


int snprintf_(char* buffer, size_t count, const char* format, ...)
printf_int_t snprintf_(char* buffer, size_t count, const char* format, ...)
{
va_list va;
va_start(va, format);
const int ret = _vsnprintf(out_buffer, buffer, count, format, va);
const printf_int_t ret = _vsnprintf(out_buffer, buffer, count, format, va);
va_end(va);
return ret;
}


int vprintf_(const char* format, va_list va)
printf_int_t vprintf_(const char* format, va_list va)
{
char buffer[1];
return _vsnprintf(&out_putchar, buffer, (size_t)-1, format, va);
}

int vsprintf_(char* buffer, const char* format, va_list va)
printf_int_t vsprintf_(char* buffer, const char* format, va_list va)
{
return _vsnprintf(out_buffer, buffer, (size_t)-1, format, va);
}

int vsnprintf_(char* buffer, size_t count, const char* format, va_list va)
printf_int_t vsnprintf_(char* buffer, size_t count, const char* format, va_list va)
{
return _vsnprintf(out_buffer, buffer, count, format, va);
}


int fctprintf(void (*out)(char character, void* arg), void* arg, const char* format, ...)
printf_int_t fctprintf(void (*out)(char character, void* arg), void* arg, const char* format, ...)
{
va_list va;
va_start(va, format);
const int ret = vfctprintf(out, arg, format, va);
const printf_int_t ret = vfctprintf(out, arg, format, va);
va_end(va);
return ret;
}

int vfctprintf(void (*out)(char character, void* arg), void* arg, const char* format, va_list va)
printf_int_t vfctprintf(void (*out)(char character, void* arg), void* arg, const char* format, va_list va)
{
const out_function_wrapper_type out_fct_wrap = { out, arg };
return _vsnprintf(out_wrapped_function, (char*)(uintptr_t)&out_fct_wrap, (size_t)-1, format, va);
Expand Down
32 changes: 24 additions & 8 deletions src/printf/printf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#ifdef __cplusplus
# include <cstdarg>
# include <cstddef>

extern "C" {
#else
# include <stdarg.h>
Expand Down Expand Up @@ -70,6 +71,21 @@ __attribute__((format(__printf__, (one_based_format_index), (first_arg))))
# define vprintf_ vprintf
#endif

#if !(defined(PRINTF_CUSTOM_INT_64) || defined(PRINTF_CUSTOM_INT_32) \
|| defined(PRINTF_CUSTOM_INT_16))
typedef int printf_int_t;
typedef unsigned int printf_uint_t;
#elif defined(PRINTF_CUSTOM_INT_64)
typedef int64_t printf_int_t;
typedef uint64_t printf_uint_t;
#elif defined(PRINTF_CUSTOM_INT_32)
typedef int32_t printf_int_t;
typedef uint32_t printf_uint_t;
#elif defined(PRINTF_CUSTOM_INT_16)
typedef int16_t printf_int_t;
typedef uint16_t printf_uint_t;
#endif

/**
* Output a character to a custom device like UART, used by the printf() function
* This function is declared here only. You have to write your custom implementation somewhere
Expand All @@ -86,7 +102,7 @@ void putchar_(char character);
* @param format A string that specifies the format of the output
* @return The number of characters that are written into the array, not counting the terminating null character
*/
int printf_(const char* format, ...) ATTR_PRINTF(1, 2);
printf_int_t printf_(const char* format, ...) ATTR_PRINTF(1, 2);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not possible. The printf() functions must be compatible with the standard library. If the MISRA standard is incompatible with using the standard library, then we have a problem.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not necessarily incompatible with the std lib. If you're not defining PRINTF_CUSTOM_INT_xx then printf_int_t is a plain int. For MISRA compatibility you'd define PRINTF_CUSTOM_INT_32 to make printf_int_t an int32_t.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, ok. I guess we can do something like that. But - I still have to see the literal int in the signature. So I would rather have something like:

#ifndef PRINTF_CUSTOM_INT_xx
int printf_(const char* format, ...) ATTR_PRINTF(1, 2);
#else
printf_int_t printf_(const char* format, ...) ATTR_PRINTF(1, 2);
#endif

or maybe even:

#ifndef PRINTF_CUSTOM_INT_xx
int def1
int def2
int def3
#else 
printf_int_t def1
printf_int_t def2
printf_int_t def3
#endif

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've update the header file. Is this better?

Copy link
Owner

@eyalroz eyalroz Jan 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need a new PR with whatever changes remain after my next push.

Also... what does MISRA have to say about the standard library version of these functions? I mean, they return a plain int, after all.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I've just read the MISRA rule regarding typedefs. It says:

6.3 (adv) typedefs that indicate size and signedness should be used in place of the basic numerical types.

So, first of all, it's a "should", not a "shall" or "will". But even more importantly - the changes you propose don't bring you much closer to satisfying this rule, since the typedef you proposed indicates neither size nor signedness. I believe this advisory is about preferring int32_t, uint8_t and such over int, unsigned, char and such.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok. I'll see about that new pull request.

Basically MISRA indicates (among others) to avoid the standard library methods regarding time/date, character handling, stream operators and allocation/deallocation. It more or less recognizes one cannot do without, but discourages dependence.

More to the point, it says "For example, the essential type rules apply to the types of the arguments passed to functions specified in the standard library and to their results."

Copy link
Author

@sanderh255 sanderh255 Jan 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I've just read the MISRA rule regarding typedefs. It says:

6.3 (adv) typedefs that indicate size and signedness should be used in place of the basic numerical types.

So, first of all, it's a "should", not a "shall" or "will". But even more importantly - the changes you propose don't bring you much closer to satisfying this rule, since the typedef you proposed indicates neither size nor signedness. I believe this advisory is about preferring int32_t, uint8_t and such over int, unsigned, char and such.

I completely agree that is what the rule is about.
That's what I'm doing by defining printf_int_t as int16_t , int32_t or int64_t. This way the return value uses a type with defined signedness and size.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically MISRA indicates (among others) to avoid the standard library methods regarding time/date, character handling, stream operators and allocation/deallocation. It more or less recognizes one cannot do without, but discourages dependence.

Well, this library is about providing these same problematic functions. But - we'll see how it looks in the new PR. Closing this one for now.



/**
Expand All @@ -97,8 +113,8 @@ int printf_(const char* format, ...) ATTR_PRINTF(1, 2);
* @param va A value identifying a variable arguments list
* @return The number of characters that are WRITTEN into the buffer, not counting the terminating null character
*/
int sprintf_(char* buffer, const char* format, ...) ATTR_PRINTF(2, 3);
int vsprintf_(char* buffer, const char* format, va_list va) ATTR_VPRINTF(2);
printf_int_t sprintf_(char* buffer, const char* format, ...) ATTR_PRINTF(2, 3);
printf_int_t vsprintf_(char* buffer, const char* format, va_list va) ATTR_VPRINTF(2);


/**
Expand All @@ -111,8 +127,8 @@ int vsprintf_(char* buffer, const char* format, va_list va) ATTR_VPRINTF(2);
* null character. A value equal or larger than count indicates truncation. Only when the returned value
* is non-negative and less than count, the string has been completely written.
*/
int snprintf_(char* buffer, size_t count, const char* format, ...) ATTR_PRINTF(3, 4);
int vsnprintf_(char* buffer, size_t count, const char* format, va_list va) ATTR_VPRINTF(3);
printf_int_t snprintf_(char* buffer, size_t count, const char* format, ...) ATTR_PRINTF(3, 4);
printf_int_t vsnprintf_(char* buffer, size_t count, const char* format, va_list va) ATTR_VPRINTF(3);


/**
Expand All @@ -121,7 +137,7 @@ int vsnprintf_(char* buffer, size_t count, const char* format, va_list va) ATTR_
* @param va A value identifying a variable arguments list
* @return The number of characters that are WRITTEN into the buffer, not counting the terminating null character
*/
int vprintf_(const char* format, va_list va) ATTR_VPRINTF(1);
printf_int_t vprintf_(const char* format, va_list va) ATTR_VPRINTF(1);


/**
Expand All @@ -133,8 +149,8 @@ int vprintf_(const char* format, va_list va) ATTR_VPRINTF(1);
* @param va A value identifying a variable arguments list
* @return The number of characters that are sent to the output function, not counting the terminating null character
*/
int fctprintf(void (*out)(char character, void* arg), void* arg, const char* format, ...) ATTR_PRINTF(3, 4);
int vfctprintf(void (*out)(char character, void* arg), void* arg, const char* format, va_list va) ATTR_VPRINTF(3);
printf_int_t fctprintf(void (*out)(char character, void* arg), void* arg, const char* format, ...) ATTR_PRINTF(3, 4);
printf_int_t vfctprintf(void (*out)(char character, void* arg), void* arg, const char* format, va_list va) ATTR_VPRINTF(3);

#ifdef __cplusplus
}
Expand Down