Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CF Code Style and Coding standards compliance #42

Closed
jphickey opened this issue Nov 23, 2021 · 6 comments · Fixed by #98
Closed

CF Code Style and Coding standards compliance #42

jphickey opened this issue Nov 23, 2021 · 6 comments · Fixed by #98
Milestone

Comments

@jphickey
Copy link
Contributor

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1778] CF Code Style and Coding standards compliance
Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Tue Nov 16 16:40:53 2021

Original Description:
There are a number of style aspects of the CF source code that should be cleaned up to better comply with GSFC coding standards:

  • inconsistent indentation/spaces, brace/comment style, lots of lines with trailing whitespace.
  • inline variable instantiations
  • variables and other symbols at global scope not properly named

Recommendations:

  • Make a one-time pass through "clang-format" using same rules as applied to CFE framework to clean up whitespace
  • All local variables should be declared at function start
  • All global variables should be qualified with the app prefix (e.g. CF_) and ideally all application state should be consolidated into a single top-level global variable (such that it can be memset to zero if/when application restarts).
@jphickey
Copy link
Contributor Author

Imported comment by internal user on Tue Nov 16 16:43:32 2021

CF also has a number of functions that have a trailing underscore (e.g. CF_CFDP_PlaybackDir_) for no apparent reason. This should be corrected and the underscore removed

@jphickey
Copy link
Contributor Author

Imported comment by internal user on Wed Nov 17 09:16:55 2021

In addition to above, there are also function names that differ only in the presence (or not) of a trailing underscore. For example:

CF_CFDP_FindTransactionBySequenceNumber{{ }}

CF_CFDP_FindTransactionBySequenceNumber_

The former is a global (app-wide) call, where the latter is a local helper function to implement that call. However I understand coding standards to require a greater differentiation in the function names than just a trailing underscore.

@jphickey
Copy link
Contributor Author

Imported comment by internal user on Wed Nov 17 11:22:05 2021

Another specific area of concern/issue here is with regard to the "clist_node" typedef. It is defined as follows:
struct clist_node_t;
typedef struct clist_node_t

{
struct clist_node_t *next;
struct clist_node_t *prev;
} clist_node_t;
typedef struct clist_node_t *clist_node;

Typically in the CFS patterns/style the bare struct would be named "clist_node", but in this case that is a pointer typedef. In general the CFE code avoids pointer typedefs, they can be confusing - just including the "*" generally yields clearer code because its obvious the value is a pointer, and doesn't cause strangeness WRT the "const" keyword.

@jphickey
Copy link
Contributor Author

jphickey commented Dec 7, 2021

Since scrubbing for adherence to the CFS conventions for naming and style is likely to be a large changeset, this should be split into several stages:

Naming convention issues are mostly just search and replace, so little chance for unintended consequences if done properly - but they do affect many files, so unlikely to be able to merge other changes across it.

jphickey added a commit to jphickey/CF that referenced this issue Dec 7, 2021
Scrub "cf_cfdp_pdu.h" for adherence to naming conventions
and style standards.  Mostly search/replace for identifier names,
and adjust whitespace accordingly.

This also adds documentation to the structures in this file,
indicating their origin from the CCSDS blue book.
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
Updates names of all identifiers declared in cf_cfdp_pdu.h to adhere
to naming conventions.

This also updates documentation on many of the items in cf_cfdp_pdu.h
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
Updates names of all identifiers in cf_cfdp.h to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
Updates names of all identifiers in cf_chunk.h to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
Update names of all identifiers in cf_clist.h to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
Updates names of all identifiers in cf_crc.h to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
Updates names of all identifiers in cf_msg.h to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
Updates names of all identifiers in cf_timer.h and cf_utils.h to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
…fy.h

Updates names of all identifiers in cf_version.h and cf_verify.h to follow naming
conventions from CFE.

This only affects include guards
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
Updates names of all identifiers declared in cf_cfdp_pdu.h to adhere
to naming conventions.

This also updates documentation on many of the items in cf_cfdp_pdu.h
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
Updates names of all identifiers in cf_cfdp.h to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
Updates names of all identifiers in cf_chunk.h to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
Update names of all identifiers in cf_clist.h to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
Updates names of all identifiers in cf_crc.h to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
Updates names of all identifiers in cf_msg.h to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
Updates names of all identifiers in cf_timer.h and cf_utils.h to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
…fy.h

Updates names of all identifiers in cf_version.h and cf_verify.h to follow naming
conventions from CFE.

This only affects include guards
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
Updates names of all identifiers in platform_inc to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
Updates names of all identifiers in cf_timer.h and cf_utils.h to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
…fy.h

Updates names of all identifiers in cf_version.h and cf_verify.h to follow naming
conventions from CFE.

This only affects include guards
jphickey added a commit to jphickey/CF that referenced this issue Dec 8, 2021
Updates names of all identifiers in platform_inc to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 9, 2021
Updates names of all identifiers declared in cf_cfdp_pdu.h to adhere
to naming conventions.

This also updates documentation on many of the items in cf_cfdp_pdu.h
jphickey added a commit to jphickey/CF that referenced this issue Dec 9, 2021
Updates names of all identifiers in cf_cfdp.h to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 9, 2021
Updates names of all identifiers in cf_chunk.h to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 9, 2021
Update names of all identifiers in cf_clist.h to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 9, 2021
Updates names of all identifiers in cf_crc.h to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 9, 2021
Updates names of all identifiers in cf_msg.h to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 9, 2021
Updates names of all identifiers in cf_timer.h and cf_utils.h to follow naming
conventions from CFE.
jphickey added a commit to jphickey/CF that referenced this issue Dec 9, 2021
…fy.h

Updates names of all identifiers in cf_version.h and cf_verify.h to follow naming
conventions from CFE.

This only affects include guards
jphickey added a commit to jphickey/CF that referenced this issue Dec 9, 2021
Updates names of all identifiers in platform_inc to follow naming
conventions from CFE.
@jphickey
Copy link
Contributor Author

jphickey commented Dec 9, 2021

The linked PR fixes the names of identifiers in headers, but still does not address the other issues, specifically declaring variables in the middle of functions, and internal helper functions which are not declared in headers (specifically the ones that end in underscores).

I'd like to close this issue with PR #98 and I will submit additional issues for the remaining items.

astrogeco added a commit that referenced this issue Dec 9, 2021
Fix #42, naming conventions and style compliance
@jphickey
Copy link
Contributor Author

jphickey commented Dec 9, 2021

Note - issues #109 and #110 are for the items noted above but not fixed in PR #98

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants