[Nut-upsuser] Plea for a more loquacious nut

[Charles Lepple]

On Dec 4, 2013, at 7:31 PM, Charles Lepple wrote:

> On Dec 4, 2013, at 3:35 PM, Roger Price wrote:
> 
>> Looking at the source code, it seems that much of what is needed is already in place, but behind "if" conditions that ensure that little or nothing gets through.  Long ago I wrote software, including a compiler, but my C programming is limited to a class exercise many many years ago, and its based on this "experience" that I'm guessing that in upssched.c function exec_cmd the code
>> 
>> snprintf(buf, sizeof(buf), "%s %s", cmdscript, cmd);
>> err = system(buf);
>> if (WIFEXITED(err)) {
>> 	if (WEXITSTATUS(err)) {
>> 		upslogx(LOG_INFO, "exec_cmd(%s) returned %d", buf, WEXITSTATUS(err));
>> 	}
>> 
>> attempts to send a command to the operating system, possibly to execute a Bash script.  If system(buf) fails, the tests block the error message. Surely the error message is essential. An unattended box is now in an emergency situation.  After the inevitable IT failure the system should be auditable to discover what went wrong and what should be done to prevent it happening in the future.  Such an audit expects to find "exec_cmd(%s) returned %d" in the log.
> 
> Are you looking for:
> 
> * more diagnostics depending on the value of err,
> * logging of all return codes, even success
> 
> or both?

Roger,

Did you mean "If system(buf) succeeds, the tests block the success message"? The line just after what you quoted handles the case where WIFEXITED(err) == 0:

https://github.com/networkupstools/nut/blob/6a4729cfdf9df01e0e3a253678c43c2594dd51b0/clients/upssched.c#L86

Code like the following should log something in all cases:

err = system(buf);
if (WIFEXITED(err)) {
	if (WEXITSTATUS(err)) {
		upslogx(LOG_INFO, "exec_cmd(%s) returned %d", buf, WEXITSTATUS(err));
	} else {
		upslogx(LOG_DEBUG, "exec_cmd(%s) succeeded"); /* NEW */
	}
} else {
	if (WIFSIGNALED(err)) {
		upslogx(LOG_WARNING, "exec_cmd(%s) terminated with signal %d", buf, WTERMSIG(err));
	} else {
		upslogx(LOG_ERR, "Execute command failure: %s", buf);
	}
}

-- 
Charles Lepple
clepple at gmail