"Peter N. Lundblad" <peter@famlundblad.se> wrote on 03/24/2006 03:25:36
PM:
> > Index: subversion/libsvn_repos/hooks.c
> > ===================================================================
<snip>
> > + char buffer[20];
>
> This seems like a rather small buffer size.
This size goes back to an IBM code example I used to get me started, so
there is no compelling reason for it on my part. Searching through the
Subversion code base I see a wide variety of buffer sizes used. Is there
any rule of thumb for such things? Any words of wisdom appreciated.
> > + /* Allocate memory for the native_args string array plus one for
> > + * the ending null element. */
> > + native_args = apr_palloc(pool, sizeof(char *) * args_arr_size + 1);
> ^ ^
> Missing parens. Above is interpreted as (sizeof(char*) * args_arr_size)
+ 1.
> You're missing _() marks for translation here and in other places, but
> if your port doesn't use gettext, then you're just doing us
> translators a favour:-)
We don't support translation as of yet on the iSeries port so I removed
all the _() marks per Julian's recommendation to do it all one way or the
other.
> > + cmd);
> > + }
> > +
> > + /* If there is "APR stdin", read it and then write it to the hook's
> > + * stdin pipe. */
> > + if (stdin_handle)
> > + {
> > + while (1)
> > + {
> > + apr_size_t bytes_read = sizeof(buffer);
> > + int wc;
> > + svn_error_t *err = svn_io_file_read(stdin_handle, buffer,
> > + &bytes_read, pool);
> > + if (err && APR_STATUS_IS_EOF(err->apr_err))
> > + break;
> > +
> > + if (err)
> > + return svn_error_createf(SVN_ERR_EXTERNAL_PROGRAM, NULL,
> > + "Error piping stdin to hook "
> > + "script '%s'", cmd);
> > +
> > + wc = write(stdin_pipe[1], buffer, bytes_read);
> > + if (wc == -1)
> > + return svn_error_createf(SVN_ERR_EXTERNAL_PROGRAM, NULL,
> > + "Error piping stdin to hook "
> > + "script '%s'", cmd);
> > + }
> > +
> > + /* Close the write end of the stdin pipe. */
> > + if (close(stdin_pipe[1]) == -1)
> > + return svn_error_createf(SVN_ERR_EXTERNAL_PROGRAM, NULL,
> > + "Error closing write end of stdin
pipe "
> > + "to hook script '%s'", cmd);
> > + }
> > +
>
> This is a deadlock if the stderr pipe fills up while you're writing to
> the stdin of the hook script. You need to use poll/select to
> read/write simultaneously.
Sorry I committed this before you caught it. The deadlock scenario you
describe was easy to create, even if my example was a bit contrived. I'm
working on this now...more soon.
Paul B.
_____________________________________________________________________________
Scanned for SoftLanding Systems, Inc. and SoftLanding Europe Plc by IBM Email Security Management Services powered by MessageLabs.
_____________________________________________________________________________
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Mar 27 18:24:12 2006