Report symEach errors in OS_SymbolTableDump_Impl

[skliper]

Is your feature request related to a problem? Please describe.
Errors not reported:

osal/src/os/vxworks/src/os-impl-symtab.c

Line 262 in ead5723

(void)symEach(sysSymTbl, OS_SymTableIterator_Impl, 0);

Describe the solution you'd like
Check for non-NULL, report error (write to file?), return error.

Describe alternatives you've considered
None

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC, OSAL code review

[jphickey]

Looked up the VxWorks docs for symEach .... This function is defined a little oddly. It returns NULL on success and on an error it returns a pointer to the symbol it last visited. However as far as I can tell the only way for this to happen is for the user callback function to return false.

With the way the callback is currently implemented, all errors are currently reported at least via OS_DEBUG() and also possibly translated to an OS_ERROR response already.

It looks like this function can also return NULL if the symbol table ID itself was invalid - but this is getting the value direct from vxWorks (sysSymTbl global) and therefore cannot be bad. It's also not clear now this is differentiated from the success case, since both return NULL.

My recommendation is that we leave the call to symEach() as it is. The VxWorks API here is not ideal - return value is ambiguous (NULL can mean success or error, and also non-NULL does not necessarily mean an error). It is being handled as well as can be done with the limitations of this underlying API design.

The only question I can see is whether exceeding the size_limit should be treated as an error or not. Currently it just stops at the size limit - there is a debug statement to say the limit was reached, but the overall function will still return OS_SUCCESS. A case could be made that this should be an error.

[skliper]

Ping @klystron78 - is this response sufficient to close the issue?

[jphickey]

After reviewing the function here -- there are some better return codes we could propagate up the stack. In particular I think if the size limit was insufficient we should not be returning OS_SUCCESS.

I will submit a PR to address that shortly.