2

I have written a simple function to initialise the structure values using memset(). These are the code I have written in C language.

myfile.h

typedef struct{
 bool flag;
 bool check;
 int val_1;
 int val_2;
} MY_STRUCT;

myfile.c

static MY_STRUCT mystruct;

void Test()
{
  memset(&mystruct, 0, sizeof(MY_STRUCT)); 
}

When i run MISRA , i am getting this kind of error

The return value of non-void function 'memset' shall be used

I have tried to fix this warning using below method

(void)memset(&mystruct, 0, sizeof(MY_STRUCT)); 

But unfortunately, i am getting 2 new warnings

Cast between types that are not both pointers or not pointers

object of pointer type 'void*' cast to unrelated type 'void'

Anybody suggest how to fix this warning while using memset() function ? Also please explain to avoid such warnings in future.

user2986042
  • 1,098
  • 2
  • 16
  • 37
  • Well, MISRA is an overly restrictive coding standard. This is asking you to check the return of `memset` and handle it in some way. So just do it (yes, pretty stupid as it has no use in your case). – Eugene Sh. Apr 11 '22 at 15:36
  • `if (memset(...) == &mystruct)`... – Barmar Apr 11 '22 at 15:43
  • @Barmar Is there any use-case where it would be false? Except some undefined behavior. – Eugene Sh. Apr 11 '22 at 15:44
  • @EugeneSh. No. In fact, it should probably invert the test and then report that the library is broken. – Barmar Apr 11 '22 at 15:45
  • You could make a macro, like `#define MYMEMSET(ptr, value, len) do {void *dummy = (void*)memset(ptr, value, len);} while(0)` and then use MYMEMSET instead of `memset`. – Jabberwocky Apr 11 '22 at 15:50
  • @Jabberwocky I expect it to complain about unused `dummy` :) – Eugene Sh. Apr 11 '22 at 15:51
  • @EugeneSh. or `#define MYMEMSET(ptr, value, len) do { if (memset(ptr, value, len) != NULL) {} } while (0)`, but then it might complain about an empty `{}`. – Jabberwocky Apr 11 '22 at 15:55
  • 2
    Unrelated, but there is no need to zero-initialize a `static` struct. – Eugene Sh. Apr 11 '22 at 15:56
  • I don't have a MISRA checker available but, at the risk of invalidating my own answer, does this work: `(void)*memset(&mystruct, 0, sizeof(MY_STRUCT));` (adding a `*`)? It's legal/valid C. – Adrian Mole Apr 11 '22 at 16:24
  • @AdrianMole When became dereferencing a `void*` legal C? – Gerhardh Apr 11 '22 at 16:27
  • @EugeneSh. you haven't yet been asked to comply to HIS metric limits, have you? :) – Gerhardh Apr 11 '22 at 16:29
  • 1
    @Gerhardh Thanks god, it's the first time I hear about these :P – Eugene Sh. Apr 11 '22 at 16:30
  • It adds to the pain if you work in an environments where you cannot afford spending extra code just to make the MISRA checker happy. If your MCU only comes with 32kB of Flash you don't want to waste a single byte for such useles extra handling. – Gerhardh Apr 11 '22 at 16:34
  • Curious: does `(void) !memset(...)` still produce the second warning? – Oka Apr 11 '22 at 16:44
  • @Gerhardh Hmm. Fair comment... weird that clang doesn't flag it though. – Adrian Mole Apr 11 '22 at 16:46
  • 1
    Better use `sizeof mystruct` instead of `sizeof (MY_STRUCT)`. Second is the size of structure type and first is size of actual structure. In theory they are the same but first is for the specific structure by name. – i486 Apr 11 '22 at 16:59
  • 1
    @EugeneSh. It's common practice in safety related software not to rely on zero-initialization of `.bss`, which could have happened weeks, months or years before those variables were used for the first time. RAM isn't trusted to hold values that long (plus bugs could corrupt memory too). Instead variables are typically initialized from flash in runtime, at the point before they are about to be used. – Lundin Apr 12 '22 at 06:25
  • 1
    "Making a MISRA checker happy" usually means fixing your broken code... – Andrew Apr 12 '22 at 08:08
  • 2
    Btw `void Test()` is probably not MISRA compliant since a visible prototype-format function declaration is required, with the same signature as the function definition. So I believe that the function must be `void Test (void)`. – Lundin Apr 12 '22 at 09:33
  • @Andrew that is a bit too simple. At lease Coverity creates tons of findings for rule 4.9 where the use of a macro is not at all interchangeable with using a function. Or Rule 8.7 that is able to completely spoil project file structure. – Gerhardh Apr 12 '22 at 13:09
  • Directive 4.9 is *Advisory* so selectively *Disappy* it if it is a problem... Rule 8.7 violations are easy to fix... – Andrew Apr 12 '22 at 21:49

3 Answers3

2

You need to understand the rationale behind the rules if you are to work with MISRA-C.

The first problem should indeed be solved by casting the result to (void). The reason for this rule is to enforce error checking of returned values from functions, which doesn't apply in this case, since memset's admittedly weird API is simply returning the first parameter.

The rest of the warnings you get seem to be false positives by your tool. It is compliant to explicitly cast the return value of any function to (void), see MISRA C:2012 17.7.

A work-around isn't required for MISRA compliance, though if you just wish to silence the tool, then this would be MISRA compliant too, and equivalent to memset:

mystruct = (struct MY_STRUCT){ 0u };
Lundin
  • 195,001
  • 40
  • 254
  • 396
1

The return value of the memset function is a void* pointer, which is a copy of the passed first parameter (dest). In your case, that is the address of an actual variable, so it cannot feasibly be NULL; thus, comparing the returned value to NULL would most likely prevent the MISRA warning (but that test should really never fail):

if (!memset(&mystruct, 0, sizeof(MY_STRUCT))) {
    (void)fputs("I'm sorry, but an impossible error has occurred!\n", stderr);
} 
Adrian Mole
  • 49,934
  • 160
  • 51
  • 83
  • 2
    `(void)memset(&mystruct, 0, sizeof(MY_STRUCT));` is already fine and MISRA compliant, no need to do anything. The problem is (as always) with the static analyser tool and not with MISRA C. This is a problem when you take someone used at only dealing with compiler diagnostics on some PC system and put them in a MISRA C project: they are used at getting high quality and nearly always accurate diagnostics. But the various MISRA checkers out there are of diverse quality, made by semi-sketchy, unprofessional private companies, so they give out tons of false positives. – Lundin Apr 12 '22 at 06:39
1

The cited code:

static MY_STRUCT mystruct;

void Test()
{
  memset(&mystruct, 0, sizeof(MY_STRUCT)); 
}

Is a violation of MISRA C:2012 Required Rule 17.7 which states, unambiguously, that The value returned by a function having non-void return type shall be used.

memset() returns a void* and therefore to comply with Rule 17.7 it must be used or as suggested by the Rule, cast to void.

While in the specific case of memset() it could be argued that the return value is pointless the language is the language and the MISRA Rule does not have an exception.

Adding a cast to (void) should be no overhead... but if you are specially concerned, you could always deviate the Rule instead.

Andrew
  • 2,046
  • 1
  • 24
  • 37