0

I am getting below MISRA QAC Warning.

This assignment is redundant. The value of this object is never used before being modified. MISRA_2012, QAC, Message Identifier: 2982

I am trying to modify local status of variable to specific error value.

code:

FUNC(void, RTE_CODE) Rte_Server_S_service_exteriorLighting_UI_unsubscribeExteriorLightSettings (ExteriorLightingUI_unsubscribeExteriorLightSettings_subscriptionId_type subscriptionId, P2CONST(uint8, AUTOMATIC, RTE_APPL_DATA) consumerId, P2VAR(ExteriorLightingUI_AT_SubscriptionStatus, AUTOMATIC, RTE_APPL_DATA) status)
{
  uint16 localStatus;
  TS_MemSet(&localStatus, 0u, sizeof(localStatus));
  uint8 conId;
  uint8 isSubIdFound = COMH_FALSE;
  
  if(subscriptionId == COMH_SUBSCRIPTION_TO_ALL)
  {
      conId = SubAll_ES_is_validate_ConsumerId(consumerId); //function call 
      if(conId < 2u)
      {
          localStatus = (uint16) COMH_SOMEIP_SUBSCRIPTION_CANCELLED;
          isSubIdFound = COMH_TRUE;
      }
      else
      {
          localStatus = (uint16) COMH_SOMEIP_SUBSCRIPTION_ERROR;
      }
  }
  else
  {
    localStatus = (uint16) COMH_SOMEIP_SUBSCRIPTION_ERROR;
  }

  if(isSubIdFound ==  COMH_FALSE)
  {
        localStatus = (uint16) COMH_SOMEIP_SUBSCRIPTION_TARGET_DELETED;
  }

  /* fill response buffer of SOMEIP method */
  TS_MemCpy(status, &localStatus, sizeof(localStatus));
}

Before this statement I am using memset to fill 0 value in localStatus.
After this statement, I am using memcpy to fill respected error code in localStatus.

kapilddit
  • 1,729
  • 4
  • 26
  • 51
  • @Mat : Purpose of this code is to load the local status with different error depending on certain if condition. and at the end it should copy local status to status which was passed as argument to the function. – kapilddit Jul 06 '22 at 11:12
  • 2
    Unrelated to the problem, using your own home-brewed integer types instead of `stdint.h` is just bad and dangerous practice. In the case you are stuck with C90 then simply `typedef` some types with the same names as those in `stdint.h`. – Lundin Jul 06 '22 at 11:19
  • 1
    In the code you have now, multiple assignments to `localStatus` are clearly redundant. If any branch except the one setting `isSubIdFound = COMH_TRUE;` is taken, then the last `if` overwrites it with `COMH_SOMEIP_SUBSCRIPTION_TARGET_DELETED`. – user17732522 Jul 06 '22 at 11:34
  • @user17732522 : Yes, its redundant. I removed.. – kapilddit Jul 06 '22 at 11:40

1 Answers1

1

Very likely the cause is TS_MemCpy(status, &localStatus, sizeof(localStatus)); which should be &status. If so, this should not just be a MISRA violation but a C compiler error.

There might also be other reasons: the static analyser might not be able to resolve these various functions since you run it file by file and not at the whole project at once. Static analysers then tend to whine when passing uninitialized variables to functions, but that's a false positive most of the time.

Or if the static analyser is really smart, it is telling you that these function calls are just nonsense, which might be true. I don't know what these functions do but the code does looks fishy. You might have some valid reasons for calling them (atomic access? MISRA compliant std lib?), but generally for a plain uint16_t you should simply do:

localStatus = (uint16_t) ERROR; // cast is only required if ERROR is an enum etc
status = localStatus;
Lundin
  • 195,001
  • 40
  • 254
  • 396
  • in my case status is (uint16_t *) Pointer. That's I mention : TS_MemCpy(status, &localStatus, sizeof(localStatus)); – kapilddit Jul 06 '22 at 11:19
  • 1
    @kapilddit In the code you posted it says that status is of type `uint16`. Did you post different code than the real one or what? – Lundin Jul 06 '22 at 11:20
  • @SupportUkraine : It is complicated to add full code. so added the logic used behind the code. – kapilddit Jul 06 '22 at 11:30
  • I understood the problem. The code is really redundant. as last if will overwrite the value. Thanks @SupportUkraine – kapilddit Jul 06 '22 at 11:33