Modes

Clone Tools
  • last updated a few seconds ago
Constraints
Constraints: committers
 
Constraints: files
Constraints: dates
The variable is initialized in the init().

The variable is initialized in the init().

Yes, if they are initialized in init() which they should be, they don't need to be initialized here. This puts this static variables in the uninitialized section of the memory.

Yes, if they are initialized in init() which they should be, they don't need to be initialized here. This puts this static variables in the uninitialized section of the memory.

This might have been updated in the different Dialin branch.

This might have been updated in the different Dialin branch.

Good catch, thanks.

Good catch, thanks.

The alarm is not commented out currently, this might be due to an old commit.

The alarm is not commented out currently, this might be due to an old commit.

This was commented for testing. I un-commented it.

This was commented for testing. I un-commented it.

You cannot run a function there. Also, the init function is the proper place to initialize the value.

You cannot run a function there. Also, the init function is the proper place to initialize the value.

Yes because the staging is branched form master that is very old. At the end of this code review I will update the master branch of common.

Yes because the staging is branched form master that is very old. At the end of this code review I will update the master branch of common.

Done.

Done.

Done.

Done.

Done.

Done.

Done.

Done.

This macro has been removed.

This macro has been removed.

Done.

Done.

Done.

Done.

Looks like this needs to be addressed.

Looks like this needs to be addressed.

Magic #.

Magic #.

rpm should be in ().

rpm should be in ().

Should we check that payloadLength is at least 12 bytes (sizeof(U32) x 3)?

Should we check that payloadLength is at least 12 bytes (sizeof(U32) x 3)?

There are 2 functions in this module with this name.

There are 2 functions in this module with this name.

Are these really common? Can they be moved to sys_selftest.c?

Are these really common? Can they be moved to sys_selftest.c?

DG-DEN-11928_DG Conductivity Update
DG-DEN-11928_DG Conductivity Update
Add #ifndef RELEASE around these definitions and all other build switch related code.

Add #ifndef RELEASE around these definitions and all other build switch related code.

Add #ifndef RELEASE around this structure.

Add #ifndef RELEASE around this structure.

Add #ifndef RELEASE around this enum.

Add #ifndef RELEASE around this enum.

Should be #ifndef RELEASE around this structure.

Should be #ifndef RELEASE around this structure.

I think there should be a #ifndef RELEASE around these build switches.

I think there should be a #ifndef RELEASE around these build switches.

Dara, I keep seeing this change in every code review - Compatibility.h is deleted and Compatible.h is created. Why do we keep seeing this?

Dara, I keep seeing this change in every code review - Compatibility.h is deleted and Compatible.h is created. Why do we keep seeing this?

Does this need to be memset when we can initialize it with 0'd values above? (line 92)

Does this need to be memset when we can initialize it with 0'd values above? (line 92)

Do we need an if for an always true condition? does this branch statement need to be uncommented?

Do we need an if for an always true condition? does this branch statement need to be uncommented?