Lars Schneider <[email protected]> writes: >> On 15 Aug 2017, at 19:36, Christian Couder <[email protected]> >> wrote: >> >> In handshake_capabilities() we use warning() when a capability >> is not supported, so the exit code of the function is 0 and no >> further error is shown. This is a problem because the warning >> message doesn't tell us which subprocess cmd failed. >> >> On the contrary if we cannot write a packet from this function, >> we use error() and then subprocess_start() outputs: >> >> initialization for subprocess '<cmd>' failed >> >> so we can know which subprocess cmd failed. >> >> Let's improve the warning() message, so that we can know which >> subprocess cmd failed. >> >> Signed-off-by: Christian Couder <[email protected]> >> --- >> sub-process.c | 13 ++++++++----- >> 1 file changed, 8 insertions(+), 5 deletions(-) >> >> diff --git a/sub-process.c b/sub-process.c >> index 6edb97c1c6..6b133f8dce 100644 >> --- a/sub-process.c >> +++ b/sub-process.c >> @@ -158,7 +158,8 @@ static int handshake_version(struct child_process >> *process, >> >> static int handshake_capabilities(struct child_process *process, >> struct subprocess_capability *capabilities, >> - unsigned int *supported_capabilities) >> + unsigned int *supported_capabilities, >> + const char *cmd) >> { >> int i; >> char *line; >> @@ -184,8 +185,8 @@ static int handshake_capabilities(struct child_process >> *process, >> if (supported_capabilities) >> *supported_capabilities |= capabilities[i].flag; >> } else { >> - warning("external filter requested unsupported filter >> capability '%s'", >> - p); >> + warning("subprocess '%s' requested unsupported >> capability '%s'", >> + cmd, p); > > Wouldn't it be possible to use "process->argv[0]"? > Shouldn't that be the same as "cmd"?
It is good to see many people are in agreement and in favor of giving more information. It is even better to see somebody noticing a room for improvement ;-) I also wonder if this should be left as a silent warning that the caller cannot notice as in the corrent code. Dying here might not be desirable for some callers, but even if we don't die here, we may want to give the caller a chance to react to the protocol error.
