3

I wrote this memcpy function, but I still need to disable rules 11.5 and 11.8. Is there a solution to be full MISRA:2012 compatible?

#pragma cstat_suppress="MISRAC2012-Rule-21.6" // Uses of stdio.h were found.
#include <stdio.h>

#include <stdint.h>
#include <string.h>
#include <stdlib.h>

extern int main(void);

static int_least8_t _memcpy(void *dst, const void *src, const size_t length)
{
    #pragma cstat_disable="MISRAC2012-Rule-11.5" // A conversion from a pointer to void into a pointer to object was found.
    int_least8_t* destination = (int_least8_t*)dst;
    #pragma cstat_disable="MISRAC2012-Rule-11.8" // A cast that removes a const or volatile qualification was found.
    const int_least8_t* source = (int_least8_t*)src;
    #pragma cstat_restore="MISRAC2012-Rule-11.5","MISRAC2012-Rule-11.8"

    for (size_t i = 0; i < (length / sizeof(int_least8_t)); i++)
    {
        destination[i] = source[i];
    }
    return 0;
}

int main(void)
{
    int32_t src[32];
    int32_t dst[32];

    (void)memset(src, 0xff, sizeof(src));

    (void)_memcpy(dst, src, 128);

    for (size_t i = 0; i < (sizeof(src) / sizeof(src[0])); i++)
    {
        (void)printf("%d <=> %d\n", src[i], dst[i]);
    }

    return 0;
}

I am using IAR as a compiler and C-STAT for static analysis.

Lundin
  • 195,001
  • 40
  • 254
  • 396
nowox
  • 25,978
  • 39
  • 143
  • 293
  • 2
    Can you explain to us rules 11.5 and 11.8, so we don't have to go and look them up? You'll get more assistance if you formulate this as a programming question you see. – Joe Oct 10 '17 at 11:40
  • @Joe done, but there is something interesting in the last MISRA amendement: https://www.misra.org.uk/LinkClick.aspx?fileticket=V2wsZxtVGkE%3D&tabid=57. It seems memcpy is a special case – nowox Oct 10 '17 at 11:42
  • 2
    @nowox That amendment is not particularly interesting, someone's been digging too deep in the strict aliasing rule again. `memcpy` has no type safety and that's just how it is. In addition, `memcpy` itself plays a part in determining the effective type of an object. Yet another rule with a poor rationale. Anyway, your code is not the standard library `memcpy` anyhow, so the rule does not apply. – Lundin Oct 10 '17 at 12:07
  • 2
    Names starting with underscore are reserved for the implementation. They **may not** be used by application code. As a sidenote: why don't youi use your standard library's `memcpy` which is most likely highly optimised but some homebrew, most slowly version? – too honest for this site Oct 10 '17 at 14:07
  • Why does your `_memcpy` return 0? That will be a surprise for anyone expecting it to work like the standard `memcpy()`. And there's absolutely no need to cast away the `const` like that: `const int_least8_t* source = (const int_least8_t*)src;` should be fine. – Toby Speight Oct 10 '17 at 15:44
  • Amendment 1 gives you some extra rules to address (some of) the problems with memcpy() - but writing your own MYmemcpy() is not the answer! – Andrew Oct 17 '17 at 10:56
  • Rule 21.6 does not ban the use of stdio.h - just the I/O functions therein. It is perfectly legitimate to `#include ` to access NULL, EOF, size_t etc – Andrew Nov 13 '17 at 07:16
  • @Andrew NULL and size_t are defined in `stddef.h`. And I can't really see a use for EOF if you aren't using the stdio I/O functions. So there should never be a reason to include `stdio.h` in a MISRA-C compliant program. – Lundin Nov 13 '17 at 07:56
  • I agree Lundin... however......... – Andrew Nov 17 '17 at 14:09

1 Answers1

7

There is no way you can write memcpy with the standard format and be fully MISRA compliant. As you seem to have noticed, MISRA doesn't allow restrict. But there's also the rule 11.5.

Rule 11.5 regarding casts from pointer-to-void to pointer-to-type is just too cumbersome to follow in practice. It is an Advisory rule so I would just skip it. You don't need to raise a deviation.

Rule 11.8 regarding casting away qualifiers is a sound one however (and Required). There is no reason why you should do this in this case. There is a bug in your code which was prevented by MISRA. Change the code to

const int_least8_t* source = (const int_least8_t*) src;

Additional notes:

  • You need not provide a function declaration to main().
  • stdio.h is not allowed by MISRA-C.
  • Avoid declaring identifiers starting with an underscore, see C11 7.1.3.
  • There's no apparent benefit of using int_least8_t here. In addition, signed types are problematic. I would use uint8_t instead.
Lundin
  • 195,001
  • 40
  • 254
  • 396
  • There's also a potential alignment issue with using `int_least8_t`. There's no guarantee the memory being copied is aligned to the type. I'd say use `unsigned char` because under any circumstances it could be different form `uin8_t` then `usigned char` would be the right answer. – Persixty Oct 10 '17 at 12:25
  • 2
    @Persixty MISRA-C encourages the use of `stdint.h`, if you don't use the stdint types, you need a good reason. This is not a good reason - in case you have a system that doesn't support `uint8_t` but have 16 bit char, then you have some dysfunctional DSP. Which you should code in assembler and not in C. There is absolutely no need for the average C program to provide portability to oddball, exotic, obsolete DSPs. People who do that are simply wasting everyone's time. Providing portability to real-world mainstream computer architectures is sufficient. – Lundin Oct 10 '17 at 12:34
  • I wasn't aware of that. I agree it's wildly theoretical that somewhere in the motor industry someone uses something not 8-bit-byte (but I hereby don't put it past Renault or Fiat). In the face of that (frankly draconian) rule I agree no good reason. But the alignment point remains. Casting `void *` to aligned types has another issue you didn't flag. – Persixty Oct 10 '17 at 12:42
  • @Persixty Copying any `uint8_t` (or character type) to another variable of the same type is perfectly fine. Alignment issues arise when you try to access the pointed-at data as some other type, in which case you also get strict aliasing problems - and this isn't allowed by MISRA. Anyway, misalignment is no concern of "my_memcpy" in itself, but rather a concern from the caller. Apart from the fact that a copy based on aligned word size would have resulted in much faster code on some systems, rather than byte by byte. – Lundin Oct 10 '17 at 12:48
  • (It might be a good idea to invent something like `void fastcpy (uint_fast8_t* dst, const uint_fast8_t* src, size_t size)` in addition to "my_memcpy".) – Lundin Oct 10 '17 at 12:50
  • `uint8_t` will be fine. It's `int_least8_t` in the original code that's got a **theoretical** problem. If it were (unlikely) that wasn't an alias of `int8_t` then `destination[i] = source[i];` could in principle use aligned instructions and fail. Remember none of this is realistic on any modern platform because `int_least8_t` is going to be `int8_t` is going to be `signed char` is the unit of memory! – Persixty Oct 10 '17 at 13:10
  • 1
    `uint_fast8_t` may well need to address alignment. `uint_fast8_t` is the fastest type with at least 8 bits. May be wider and aligned on a optimized platform. It's most likely to use aligned instructions - on account of being advertised as fast! You can write `my_memcpy()` to do first odd bytes as `uint8_t`, the body as a wider type and last bytes as `uint8_t`. – Persixty Oct 10 '17 at 13:15
  • @Lundin If I don't forward declare main I get a MISRA error 8.4: `[MISRAC2012-Rule-8.4]:Definition of externally-linked...` perhaps it is a bug in IAR C-STAT... – nowox Oct 10 '17 at 18:48
  • @nowox - you need to raise that with IAR... main() does not require a prototype :-) – Andrew Nov 13 '17 at 07:13