On 08/05/2019 15:37, sgehw...@redhat.com wrote:
:
stripBinary uses ProcessBuilder to launch objdump but it doesn't
redirect or read from the process streams. This could make diagnosing
problems difficult so I think it should minimally use
redirectOutput(INHERIT) and redirectError(INHERIT) so that any
output/errors can be seen by the jlink user.
OK. Done.
Good, that should help in the event that objdump produces an error.



At L180 there is a question to the reader on whether arg can be null. It
could be good to answer that if possible and avoid needing to think
about that.
Answered. It must never be null currently. Once it is, it's a bug.
Hence, IllegalStateException being thrown.
It might be better to throw InternalError as ISE suggests there is a state where it could succeed.



:
FWIW, since DefaultStripDebugPlugin is being run via IntegrationTest
running the real plugins is covered too.

Thanks for the explanation. I looked through 10/webrev and I think this patch is ready to go.

-Alan

Reply via email to