CF method CF_CList_Remove appears to accept bad arguments

[jphickey]

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1719] CF method CF_CList_Remove appears to accept bad arguments
Originally submitted by: Gibson, Alan S. (GSFC-5870) on Thu Aug 26 13:23:38 2021

Original Description:
A bad node is passed to CF_CList_Remove, but it carries on unaware. This was found because branch 3 of if((node->next==node)&&(node->prev==node)) can only be covered by a test that passes this situation, node->next == node, node->prev != node.

The issue: should a single node enter, that has a node->next == node (meaning: pointing to itself), but a node->prev != node (meaning: NOT pointing to itself), the code continues as if this is a valid state. It may be impossible for a bad node to enter here, but that is not apparent at the unit test level for CF_CList_Remove nor is the Doxygen brief clear on that assumption.

[skliper]

Note is initialized as node->next = node-prev = node here:

CF/fsw/src/cf_clist.c

Lines 43 to 47 in 593d61a

	void CF_CList_InitNode(CF_CListNode_t *node)
	{
	node->next = node;
	node->prev = node;
	}

Node insert front either keeps as is (first) or updates both:

CF/fsw/src/cf_clist.c

Lines 68 to 69 in 593d61a

	node->next = *head;
	node->prev = last;

Note insert back either sets both or uses as is (first):

CF/fsw/src/cf_clist.c

Lines 101 to 104 in 593d61a

	node->next = *head;
	(*head)->prev = node;
	node->prev = last;
	last->next = node;

Insert after, same pattern:

CF/fsw/src/cf_clist.c

Lines 185 to 188 in 593d61a

	after->next = start->next;
	start->next = after;
	after->prev = start;
	after->next->prev = after;

There is no case where only one of next or prev == node possible in the code, since these are internal functions there's no requirement to protect from an impossible case. Closing as wontfix.

[skliper]

Actually, makes more sense to me to remove the redundant conditional... just check if next == node which then removes the need for the strange test.