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

Scrub OSAL for direct array references #648

Closed
jphickey opened this issue Nov 10, 2020 · 0 comments · Fixed by #666 or #680
Closed

Scrub OSAL for direct array references #648

jphickey opened this issue Nov 10, 2020 · 0 comments · Fixed by #666 or #680
Assignees
Milestone

Comments

@jphickey
Copy link
Contributor

Is your feature request related to a problem? Please describe.
In OSAL there are quite a few places with direct array references to an index, such as:

if (OS_stream_table[local_id].socket_type != OS_SocketType_STREAM)

In many functions this is repeated many times over (i.e. makes several accesses into the table entry for the given item).

Describe the solution you'd like
This should be separated to use local pointer(s) to the entry/entries in use.

First do a lookup, e.g.:

stream = OSAL_TABLE_ENTRY(OS_stream_table, local_id);

Then use stream-> to refer to that entry from there on, e.g.:

if (stream->socket_type != OS_SocketType_STREAM)

Additional context
This makes the code a lot more readable and more maintainable. The CFE was already scrubbed for this, so it makes sense for OSAL to also do the same.

Requester Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey self-assigned this Nov 10, 2020
jphickey added a commit to jphickey/osal that referenced this issue Dec 1, 2020
Introduce the OS_object_token_t type which tracks a reference
to an OSAL resource.  This contains all information about the
original reference, including the ID, object type, lock type, and
the table index.

Therefore, since the token type contains all relevant info, it
can be used in all places where a bare index was used.

This also considerably simplifies the code, as some functions
which previously output multiple objects only need to operate on
tokens, and the functions which called these functions only need
to instantiate a token.
jphickey added a commit to jphickey/osal that referenced this issue Dec 1, 2020
Introduce the OS_object_token_t type which tracks a reference
to an OSAL resource.  This contains all information about the
original reference, including the ID, object type, lock type, and
the table index.

Therefore, since the token type contains all relevant info, it
can be used in all places where a bare index was used.

This also considerably simplifies the code, as some functions
which previously output multiple objects only need to operate on
tokens, and the functions which called these functions only need
to instantiate a token.
jphickey added a commit to jphickey/osal that referenced this issue Dec 1, 2020
Use a variable name that matches the type of resource being accessed,
rather than just "local".  In particular this is important for readability
of timecb and timebase code where functions need often need to access both
types of objects at the same time.  This also updates filesys code to match.
jphickey added a commit to jphickey/osal that referenced this issue Dec 1, 2020
Use a variable name that matches the type of resource being accessed,
rather than just "local".  In particular this is important for readability
of timecb and timebase code where functions need often need to access both
types of objects at the same time.  This also updates filesys code to match.
@skliper skliper added this to the 6.0.0 milestone Dec 1, 2020
jphickey added a commit to jphickey/osal that referenced this issue Dec 2, 2020
Introduce the OS_object_token_t type which tracks a reference
to an OSAL resource.  This contains all information about the
original reference, including the ID, object type, lock type, and
the table index.

Therefore, since the token type contains all relevant info, it
can be used in all places where a bare index was used.

This also considerably simplifies the code, as some functions
which previously output multiple objects only need to operate on
tokens, and the functions which called these functions only need
to instantiate a token.
jphickey added a commit to jphickey/osal that referenced this issue Dec 2, 2020
Use a variable name that matches the type of resource being accessed,
rather than just "local".  In particular this is important for readability
of timecb and timebase code where functions need often need to access both
types of objects at the same time.  This also updates filesys code to match.
jphickey added a commit to jphickey/osal that referenced this issue Dec 2, 2020
Pass token objects to impl layers to identify the object to operate
on, rather than the index.  The token has extra information including
the original/actual object ID - which matters if the ID might change
as part of the operation (e.g. new/delete).
jphickey added a commit to jphickey/osal that referenced this issue Dec 2, 2020
Pass token objects to impl layers to identify the object to operate
on, rather than the index.  The token has extra information including
the original/actual object ID - which matters if the ID might change
as part of the operation (e.g. new/delete).
jphickey added a commit to jphickey/osal that referenced this issue Dec 2, 2020
Introduce the OS_object_token_t type which tracks a reference
to an OSAL resource.  This contains all information about the
original reference, including the ID, object type, lock type, and
the table index.

Therefore, since the token type contains all relevant info, it
can be used in all places where a bare index was used.

This also considerably simplifies the code, as some functions
which previously output multiple objects only need to operate on
tokens, and the functions which called these functions only need
to instantiate a token.
jphickey added a commit to jphickey/osal that referenced this issue Dec 2, 2020
Use a variable name that matches the type of resource being accessed,
rather than just "local".  In particular this is important for readability
of timecb and timebase code where functions need often need to access both
types of objects at the same time.  This also updates filesys code to match.
jphickey added a commit to jphickey/osal that referenced this issue Dec 2, 2020
Pass token objects to impl layers to identify the object to operate
on, rather than the index.  The token has extra information including
the original/actual object ID - which matters if the ID might change
as part of the operation (e.g. new/delete).
jphickey added a commit to jphickey/osal that referenced this issue Dec 4, 2020
Pass token objects to impl layers to identify the object to operate
on, rather than the index.  The token has extra information including
the original/actual object ID - which matters if the ID might change
as part of the operation (e.g. new/delete).
astrogeco added a commit that referenced this issue Dec 9, 2020
Fix #648, 649, 664 - refactor all table array access
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants