File operations in CFE_ES_ShellOutputCommand() need cleanup

[skliper]

This function can be simplified and also made more robust.

• Does unnecessary copying of the input parameters.

• Does NOT always ensure proper null termination of the inputs. (Copying strings could be justified if it was to ensure null termination of the inputs). The "CFE_ES_ShellCmd" function that is calling this should ensure null termination but it does not.

• Matching of the "ES" special commands needs improvement - It will not run any OS command that happens to start with "ES_", and it will also not handle it correctly if one happens to be a substring of another but not a complete match.

• It does not check the return codes from either "OS_read" or "OS_write" calls and assumes they all work perfectly - the correct operation of this function, in fact, does depend on them all working perfectly. In the worst-case scenario, a benign failure of an OS_write() call could have a cascade effect causing a later OS_read() call to block indefinitely.

• The return value of OS_lseek() is not properly checked. It also relies on having an OS implementation of lseek that can accurately "measure" the size of the file. Not all filesystems/file types are seekable in this nature.

• It will not work with any "special" file or file system that doesn't support seeking or read/write file handles (such as a pipe).

• It unnecessarily extends the file on disk to add up to 3 extra spaces, which is only to coax the message generator loop into producing an extra message.

• Overall, the loop that is generating the telemetry messages can be simplified quite a bit.

• The fixed 200ms delay between messages should at least be configurable, but there is still no way to tell if buffer overflow is occurring

• There is no sequence number, and way to tell at the receiving side of one intermediate component message was lost. (Possibly outside the scope of this, and no way to really fix this without changing the binary format of the messages).

[skliper]

Imported from trac issue 53. Created by jphickey on 2015-05-22T13:39:02, last modified: 2019-03-01T15:27:58

[skliper]

Trac comment by jphickey on 2015-05-22 14:02:31:

Need CCB input on how this Shell Command is used in real life missions, and whether these issues are worth pursuing.

[skliper]

Trac comment by jphickey on 2015-05-22 14:50:09:

Another possible issue - there is no upper bound on the size of the file that it will attempt to carve up and send as telemetry messages. So if a command produces an unexpectedly large output (just a few kB with the default segment size of 64) this could cause the ES task to spin here for an extremely long period of time.

To give a real example, if the shell command produces a 1MB file, this will take nearly 1 hour to send as 64 byte telemetry chunks, continuously sent at 5 per second, and there is no way to tell it to "stop" since this task is performed by the ES task and therefore blocking it from processing subsequent commands.

[skliper]

Piling on - CFE_ES_ShellCountObjectCallback looks to be just a copy of CFE_ES_CountObjectCallback. Note ListResources is basically ListResourcesDebug except to a file.

[skliper]

Related to #484

[skliper]

#643 - Intent is to add these commands following the typical command processing model, same as the other commands that output to file. No automatic telemetry (never was a requirement) packet. Should resolve all these issues. Old version of the code deprecated per #484