ARM-software / sdm-api

Secure Debug Manager API
BSD 3-Clause "New" or "Revised" License
6 stars 0 forks source link

Normalise API symbol naming #19

Closed flit closed 2 years ago

flit commented 2 years ago

Some of the symbol names in the API header use camel case (ExampleCamelCase), while others use snake case (Example_Snake_Case). This needs to be sorted out before symbol names are frozen for v1.0.

All of the typedefs, enum and struct tags, function formal parameters, and struct fields use camel case. The later two types have a leading lower-case character.

Examples:

typedef uint32_t SDMResetType;

enum SDMArmADITransferAttributes

typedef struct SDMItemInfo {
    const char *itemShortName;
    const char *itemLongDescription;
} SDMItemInfo;

Macros, enumerators, and functions currently use snake case to varying degrees.

Enumerator examples showing differences follow.

enum SDMVersionEnum {
    SDM_CurrentMajorVersion = 0,
    SDM_CurrentMinorVersion = 1,
};

enum SDMReturnCodeEnum {
    SDM_Success = 0,
    SDM_Fail_No_Response = 1,
    SDM_Fail_Unsupported_Transfer_Size = 2,
};

enum SDMDebugArchitectureEnum {
    SDM_Arm_ADIv5 = 0,
    SDM_Nexus5001 = 2,
};

enum SDMFlags {
    SDM_IsDefaultDeviceValid = (1 << 0),
};

enum SDMResetTypeEnum {
    SDM_DefaultReset = 0,
    SDM_HardwareReset = 1,
};

enum SDMMemorySizeEnum {
    SDM_Memory8 = 8,
    SDM_Memory16 = 16,
};

enum SDMControlStateEnum {
    SDM_ControlActiveState = 0,
    SDM_ControlInactiveState = 1,
    SDM_ControlMixedState = 2
};

enum SDMFormElementFlagsEnum {
    SDM_ElementIsOptional = (1 << 0),
    SDM_TextFieldIsPassword = (1 << 8),
};

enum SDMArmADITransferAttributes {
    SDM_ArmADI_Nonsecure_Attr = 0x0001,
    SDM_ArmADI_NonPrivileded_Attr = 0x0002,
    SDM_ArmADI_Direct_Attr_Enable = 0x0080,
    SDM_ArmADI_Direct_Attr_Mask = 0x7F00,
};

enum SDMDeviceTypeEnum {
    SDM_Invalid_Device,
    SDM_ArmADI_AP_Device,
    SDM_ArmADI_MEMAP_Device,
    SDM_ArmADI_CoreSightComponent_Device,
};

As can be seen, whether and how enumerator names currently use groupings varies.


Recommendation:

  1. Change function names to remove underscore between SDM and the rest of the symbol name.
  2. Normalise enumerator names. Remove underscore after SDM and add enum type, then a single underscore to separate the enumerator description:
    • SDMControlState_Active
    • SDMResetType_Default
    • SDMDebugArchitecture_ArmADIv5
    • SDMTransferSize_8
  3. There are a small number of cases where an extra grouping is helpful, particularly for grouping by debug architecture.
    • SDMDeviceType_ArmADI_AP
    • SDMDeviceType_ArmADI_CoreSightComponent
    • SDMTransferAttr_ArmADI_Nonsecure
    • SDMTransferAttr_ArmADI_DirectAttrEnable

Questions and comments?

nadnoo01 commented 2 years ago
  1. I agree
  2. The recommendation is more readable and consistent than before if all the enum names are changed to camel snake case and I think the separation of the description in the enum name is good as this also adds to making the names more consistent.
  3. If camel snake case is being adopted then I think there is no problem having more than one grouping.
flit commented 2 years ago

Thanks for the feedback. Yes, the idea is that all enums will be renamed to have a consistent style.

henriksandin commented 2 years ago

Looks good to me.

henriksandin commented 2 years ago

I.e. approved by IAR...

flit commented 2 years ago

Closing. Symbol names have been updated in commit 5ad9ebf.