This is the specific line in APR that causes the exit value to be -1 :
https://github.com/apache/apr/blob/42fa31d7219e9371771a1549e3c63c68f74a1540/threadproc/unix/proc.c#L592

When the apr_proc_t is created in log4cxx, it is set to be
APR_PROGRAM_PATH, which in APR will call execvp().  Since 'zip' is not
installed, -1 is returned from the child process.  errno in this case
should be set to ENOENT on the failed call to execvp().

Looking at the APR code however, there is a callback which will give us the
errno value back, although it's just a straight C callback(no void* user
data parameter is provided), so I don't think it's possible to get the
proper message back from APR easily.  I think that the best thing to do in
this case is to check the exit code and throw an IOException if it is
anything but success.

Here's a quick patch for it.  Note that if 'zip' fails, the only thing that
will be printed out is log4cxx: Exception during rollover.

-Robert Middleton

On Thu, Jul 7, 2016 at 3:09 PM, Thorsten Schöning <tschoen...@am-soft.de>
wrote:

> Guten Tag Robert Middleton,
> am Donnerstag, 7. Juli 2016 um 15:26 schrieben Sie:
>
> > I have figured out why this test is failing on my machine, I don't
> > have the 'zip' utility installed by default.
>
> Good to hear it was that simple. ;-)
>
> > Due to how APR works as well, we get back an exit code of 255
> > from the child process in the apr_proc_wait call in
> > zipcompressaction.cpp.  Normally, this should be ENOENT(or
> > APR_ENOENT), but APR returns -1 if any of the exec calls fail.
>
> I'm not sure I understand this... When is 255/-1 returned in your
> case, with missing zip or if it's available and succeeded? Because you
> are writing of "exec calls fail". Did you test what your zip utility
> returns if you invoked it on a shell?
>
> I wonder if it has a reason that the exit status is not checked, maybe
> it didn't work and is just not documented?
>
> >   2. If return code is 255, throw exception from
> ZipCompressAction::execute
>
> If we check the exit status, I prefer this one, because an exception
> is thrown already for any other problem as well. Like just starting
> the command. A failing command is no difference in my mind.
>
> Mit freundlichen Grüßen,
>
> Thorsten Schöning
>
> --
> Thorsten Schöning       E-Mail: thorsten.schoen...@am-soft.de
> AM-SoFT IT-Systeme      http://www.AM-SoFT.de/
>
> Telefon...........05151-  9468- 55
> Fax...............05151-  9468- 88
> Mobil..............0178-8 9468- 04
>
> AM-SoFT GmbH IT-Systeme, Brandenburger Str. 7c, 31789 Hameln
> AG Hannover HRB 207 694 - Geschäftsführer: Andreas Muchow
>
>
Index: src/main/cpp/zipcompressaction.cpp
===================================================================
--- src/main/cpp/zipcompressaction.cpp	(revision 1751846)
+++ src/main/cpp/zipcompressaction.cpp	(working copy)
@@ -81,7 +81,9 @@
 	stat = apr_proc_create(&pid, "zip", args, NULL, attr, aprpool);
 	if (stat != APR_SUCCESS) throw IOException(stat);
 
-	apr_proc_wait(&pid, NULL, NULL, APR_WAIT);
+	int exitcode;
+	apr_proc_wait(&pid, &exitcode, NULL, APR_WAIT);
+	if (exitcode != APR_SUCCESS) throw IOException(exitcode);
 
 	if (deleteSource)
 	{

Reply via email to