Code review comment for ~dirk.zimoch/epics-base:FixShellCommands

Hi Dirk,

Good idea, a few comments about your specific changes though.

It's only the VxWorks shell (and I guess the RTEMS CEXP shell) that can pass in NULL pointers to some of these commands. Adding the NULL check to any osdEnv.c file except for libcom/src/osi/os/vxWorks/osdEnv.c doesn't make too much sense to me.

I would prefer that we not print anything in dbStateCreate() which is also an API entry point for the dbState subsystem, just return NULL for a NULL input.

Your VxWorks dlload() routine needs a check for a NULL input argument...

Finally a few comments about your Usage messages: Parentheses are not required around arguments to VxWorks or iocsh commands, they're only needed for an IOC using CEXP on RTEMS, and the people who run such IOCs know about the requirement. Parentheses take longer to type and I'd rather we not imply they're needed when they aren't. The iocsh help output shows command usage without parentheses (which are allowed there too), although I wish it prefixed the full command with "Usage:" when shown with argumemts (like you do):

epics> help dbLoadDatabase
dbLoadDatabase 'file name' path substitutions
epics> help dbLoadRecords
dbLoadRecords 'file name' substitutions
epics> help dbb
dbb 'record name'
epics> help dbtpn
dbtpn 'record name' value
epics> help epicsEnvSet
epicsEnvSet name value

That help command only puts quotes around arguments that have spaces in their name, which is different than the requirements of the VxWorks shell (but that is how the iocsh parses arguments). Your use of double-quotes around string arguments shows they are required for VxWorks though, which is fine and correct.

Thanks,

- Andrew