On 15 Mar 2012, at 3:56 PM, Brian J. France wrote:
> Could somebody review the patch below for 2.2, 2.4, and trunk?
>
> A better error message could be sent, but I am more worried about how the
> return will effect the code after it.
>
> I am thinking the file needs to be removed either via a apr_file_remove call
> or:
>
> apr_pool_cleanup_kill(stream->p, stream, tmpfile_cleanup);
>
> call, but I don't know the code well enough to know which is right and
> 2.4/trunk adds even more complexity compared to 2.2.x.
>
> Thoughts/Comments?
>
> Thanks,
>
> Brian
>
> - I am still getting more details why close is failing, but for some reason
> it is happening enough to cause issues. (responding 200, but no file)
>
> -----
>
> 2.2:
>
> Index: modules/dav/fs/repos.c
> ===================================================================
> --- modules/dav/fs/repos.c (revision 1300964)
> +++ modules/dav/fs/repos.c (working copy)
> @@ -881,6 +881,10 @@
> {
> apr_file_close(stream->f);
Would be above not be
status = apr_file_close(stream->f);
>
> + if (status != APR_SUCCESS) {
> + return dav_new_error(stream->p, MAP_IO2HTTP(status), 0, "There was a
> problem closing the stream");
> + }
> +
> if (!commit) {
> if (apr_file_remove(stream->pathname, stream->p) != APR_SUCCESS) {
> /* ### use a better description? */
>
>
> 2.24 and trunk:
>
>
> Index: modules/dav/fs/repos.c
> ===================================================================
> --- modules/dav/fs/repos.c (revision 1300964)
> +++ modules/dav/fs/repos.c (working copy)
> @@ -970,6 +970,10 @@
>
> apr_file_close(stream->f);
Same with this one.
>
> + if (status != APR_SUCCESS) {
> + return dav_new_error(stream->p, MAP_IO2HTTP(status), 0, "There was a
> problem closing the stream");
> + }
> +
> if (!commit) {
> if (stream->temppath) {
> apr_pool_cleanup_run(stream->p, stream, tmpfile_cleanup);
>
>
Regards,
Graham
--
smime.p7s
Description: S/MIME cryptographic signature
