JSC: use fixed width types

[skliper]

Originally part of trac #45 and isolated for CCB review purposes.

Replace use of native int with fixed-width int32 typedef.

[skliper]

Imported from trac issue 116. Created by jphickey on 2015-11-06T17:17:15, last modified: 2015-12-09T11:13:56

[skliper]

Trac comment by jphickey on 2015-11-09 11:56:47:

Commit [changeset:db75439] is available for CCB review, branch trac-116-jsc-fixed-width-types

[skliper]

Trac comment by jphickey on 2015-11-09 12:01:38:

I have reviewed this and a few issues should be corrected before this is accepted.

• The id field of the OS_task_internal_record_t structure must use the same type that VxWorks indicates for task IDs. This might be int or I suspect there is an official typedef for task IDs. But int32 is incorrect here.

• The creator fields should be uint32 not int32 -- these are OSAL resource identifiers and they are unsigned. The OS_FindCreator() function that fills this field returns uint32 (correctly) so the storage type should be the same.

[skliper]

Trac comment by jphickey on 2015-11-09 12:03:41:

Another note - the free field should probably be osalbool rather than int32 as well, since that is how it is actually used, being directly compared to the macros/constants TRUE and FALSE.

[skliper]

Trac comment by sstrege on 2015-11-18 15:23:22:

11/17/15 CCB:

• Verified there is not a VxWorks task ID type. Agreement to revert code change and keep the “id” variable defined in the “OS_task_internal_record_t” typedef an int vs. int32. Comment needs to be added to identify MISRA rule exemption w/ rational.

• Agreement the “free” variables should be defined as boolean types. Need to confirm all uses before making code changes.

• Agreement “creator” variables should be a uint32 vs. int32. Need to confirm all uses before making code changes

[skliper]

Trac comment by sduran on 2015-11-23 18:39:38:

• task id changed to int. I did note another type inconsistency:
  Why is OStask_id a uint32, the VxWorks type for a task id is int,
  not changing this at this time. Need to look into how task_prop->OStask_id is used.
  task_prop -> OStask_id = (uint32)OS_task_table[task_id].id;

• Changing free to type osalbool is fine. Verified all usage compres to TRUE/FALSE. Did not change cases like if(xxx.free == TRUE) to if(xxx.free)

• changed creator from int32 to uint32. creator is used as an uint32 everwhere, however, it is strange that
  uint32 OS_FindCreator() has a uint32 return type, but is operating on and really returning a int.
  int i;
  ...
  return i;
  In this case, I "fixed" the internals of OS_FindCreator(), so that the creator variable is consistently uint32 throughout.

reference commit [changeset:039441c]

[skliper]

Trac comment by jphickey on 2015-11-23 23:47:56:

The changeset looks good and I have merged it to "ic-ccb-review" ....

HOWEVER -- please check your editor settings, as there were a lot of whitespace changes, which create merge conflicts. Commit [changeset:039441c] seems to have "cleaned" a lot of spurious whitespace in addition to the changes listed.

I fully agree on the OStask_id field of the tasks properties structure. This is defined by the OSAL API and it makes two major assumptions that the a) OS-issued task identifiers are integers in nature and b) they are 32 bits or less. These assumptions are false on at least one platform I am aware of - cygwin - where the pthread_t type is actually a structure and it is larger than 32 bits.

But we cannot/should not change the type of the API structure at this time. This sounds like another place to create a new ticket about this and encourage people NOT to use the OStask_id field for anything.

[skliper]

Trac comment by jphickey on 2015-11-24 22:51:15:

Pushed commit [changeset:4d6f76a] which contains the remainder of fixed-width type changes originally in #137 (since it looks like we may not be merging that one).

NOTE: This also contains 3 cases of unused variable removal. That bit probably technically belongs in #141, but that may create extra merge conflicts if it were actually put there, so I left them in here.

[skliper]

Trac comment by sstrege on 2015-11-30 18:39:24:

Recommend the for loop "i" variables be uint32 vs. int32. These will never be negative. Same with the "NumChars" variable defined in osfilesys.c, line 992

osfileapi.c, line 727 - "status" needs to be commented with rational for using non-fixed width type.

Recommend/accept the 3 cases of unused variable removal.

[skliper]

Trac comment by sstrege on 2015-12-07 18:02:22:

Pushed commit f2a8cd5 which changes the looping "i" variables and "NumChars" variable from int32 to uint32. Also added a comment explaining rational for keeping "status" and int.

[skliper]

Trac comment by sstrege on 2015-12-07 22:04:45:

Adding link to pushed commit [changeset:f2a8cd5]

[skliper]

Trac comment by glimes on 2015-12-09 11:08:55:

Commits [changeset:4d6f76a] and [changeset:f2a8cd5] included via merge [changeset:d0a3d1e] into merge [changeset:961b061] which is now the development branch.

[skliper]

Trac comment by glimes on 2015-12-09 11:13:56:

Merge of #136 #138 #139 #141 and #142 (2015-12-01) is now development.