CODEWITHSUNDEEP
cmisraqa-c
kapilddit
kapilddit

Reputation: 1769

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

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.

Upvotes: 0

Views: 970

Answers (2)

Torsten Knodt
Torsten Knodt

Reputation: 766

Not sure whether I get something wrong in the code, but if you replace the assignments of COMH_SOMEIP_SUBSCRIPTION_ERROR to localStatus with COMH_SOMEIP_SUBSCRIPTION_TARGET_DELETED, you can remove everything around isSubIdFound .

And you can do this, because the last assignment to localStatus is redundant.

So I would say your MISRA checker is right.

Upvotes: 0

Lundin
Lundin

Reputation: 213862

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;

Upvotes: 2

Related Questions