Duplicate OSAL error codes and error string API

[skliper]

osapi-os-filesys.h has its own set of error codes that overlap with and are different from the rest of OSAL error codes. For example, "OS_FS_ERR_NAME_TOO_LONG" is not the same as "OS_ERR_NAME_TOO_LONG". There are several codes that are redefined differently. At a minimum, this is confusing, but it can also cause real bugs if the wrong action is taken due to misinterpreting an error.

Furthermore, FS has a second implementation of OS_GetErrorName called OS_FS_GetErrorName() that only translates the FS error codes. Passing an error code from one of the filesystem functions into OS_GetErrorName will get the wrong string.

This should be cleaned up - the FS error codes should be merged with the rest of OSAL error codes into a single set, with a single implementation of OS_GetErrorName() to get them all.

[skliper]

Imported from trac issue 11. Created by jphickey on 2015-01-13T11:58:14, last modified: 2015-11-20T16:22:16

[skliper]

Trac comment by jphickey on 2015-01-13 16:02:07:

Pushed [changeset:41e4ea0] (branch trac-11-consolidate_osal_errors) as first part of this fix. This is a simple change and simply changes the FS error codes to eliminate confusion with OSAL error codes. No names were changed so as to preserve source code compatibility.

This does not consolidate the OS_GetErrorName() and OS_FS_GetErrorName() into a single function yet. To do this would require implementing the unified function separately in all OS layers. However once trac #28 is fixed then a single implementation can be done for all.

[skliper]

Trac comment by glimes on 2015-01-29 15:14:09:

Click [changeset:24cc035 here] to see the changeset.

[skliper]

Trac comment by glimes on 2015-01-29 16:02:44:

I have reviewed [changeset:24cc035 changeset 24cc035] and recommend ACCEPT for including this change in the mainline development branch, and am agnostic on whether to leave this ticket open to capture later cleanup, or to use a separate ticket for that work. Either way is good.

[skliper]

Trac comment by mlogan on 2015-01-30 19:28:13:

I have reviewed the changes and ACCEPT.

[skliper]

Trac comment by jwilmot on 2015-02-11 12:34:58:

I have reviewed the changes and ACCEPT conditionally. What is the impact of this change, is it really priority "major"? Could we deprecate OS_FS_GetErrorName and have a macro that calls OS_GetErrorName?

[skliper]

Trac comment by jphickey on 2015-02-11 13:02:09:

From a "user code" standpoint, this change is designed to be totally transparent, as long as the code used the #define symbols rather than the actual numeric values. However I have not come across ANY code that actually checked for e.g. a response of "-3" -- if someone actually did this then the code would break. Checking for "<0" is OK since the values are still negative.

The next iteration here would certainly be to unify the OS_GetErrorName and OS_FS_GetErrorName, deprecating the latter one. This would be a follow-on change. The reason this was NOT done is because OS_GetErrorName and OS_FS_GetErrorName have 3 implementations each and I am unable to build/test them all.

With Trac #28, the 3 different OS_GetErrorNames will become one and this will be much easier to clean up. (Side note - I have an implementation for #28 but getting hung up on backward/forward compatibility with unit tests - the fix for #40 will help this)

[skliper]

Trac comment by acudmore on 2015-02-11 14:17:56:

I have reviewed the changes and ACCEPT.

I'm glad to see these error codes getting cleaned up.

[skliper]

Trac comment by glimes on 2015-02-12 15:09:39:

Accepted by #41 for integration candidate.

[skliper]

Trac comment by glimes on 2015-03-04 15:20:09:

Confirmed that the fix from [changeset:24cc035] is present in the
current development branch of OSAL, [changeset:5aae372].

Given that we have bookkeeping claiming that "Ticket #34 has been integrated"
it would be good practice to open a new ticket for future work on this.