swebb2066 commented on code in PR #715:
URL: https://github.com/apache/logging-log4cxx/pull/715#discussion_r3377685859
##########
src/main/cpp/gzcompressaction.cpp:
##########
@@ -153,14 +153,20 @@ bool GZCompressAction::execute(
LOG4CXX_EXECUTE_ACTION_FORMAL_PARAMETERS ) const
return true;
}
- apr_proc_wait(&pid, NULL, NULL, APR_WAIT);
+ int exitCode = 0;
+ apr_proc_wait(&pid, &exitCode, NULL, APR_WAIT);
stat = apr_file_close(child_out);
if (stat != APR_SUCCESS)
{
throw IOException(stat);
}
+ if (exitCode != APR_SUCCESS)
+ {
+ throw IOException(exitCode);
Review Comment:
The `IOException` parameter is passed to `apr_strerror` to produce a user
intelligable message. `apr_strerror` does not expect process an exit code.
`zipcompressaction.cpp` is not a good exemplar for this.
I suggest you add an Exception type, say SubProcessFailure, whose
constructor takes a process name, exit code and reason `apr_exit_why_e`
```
apr_proc_wait(&pid[i], &exitCode, &reason, APR_WAIT);
if (exitCode != 0)
{
throw SubProcessFailure("gzip", exitCode, reason);
}
```
where SubProcessFailure::makeMessage may do something like:
```
{
std::string msg = processName;
msg += " exit code: ";
msg += std::to_string(exitCode);
if (APR_PROC_SIGNAL == reason || APR_PROC_SIGNAL_CORE ==
reason)
msg += "; exited due to a signal";
return msg;
}
```
This would allow better code reuse.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
