I'm really not that familiar with apr. Perhaps Bert can provide an
updated patch as he seems to be much more familiar with apr than I
am. Or the original change that caused the problem in the first place:
http://svn.collab.net/viewvc/svn?view=revision&revision=35533
could just be reverted.
Kyle
On May 7, 2009, at 08:38, Hyrum K. Wright wrote:
> Kyle,
> Could you provide another patch which addresses Bert's concerns?
> I'm hesitant to nominate r37615 for 1.6.x based upon this feedback.
>
> -Hyrum
>
> On May 7, 2009, at 9:53 AM, Bert Huijben wrote:
>
>> Hi Kyle,
>>
>> (Sorry for top posting, but my mail client doesn’t allow me to add
>> something between the other parts)
>>
>> Apr internally never checks a header before calling exit(). It just
>> checks the define before #including the specific header named
>> stdlib.h, so it requires exit() to be inside stdlib.h or any
>> already included header.
>>
>> Apr defines apr_proc_fork() as a portable way to call fork
>> (declared in <apr_thread_proc.h> when APR_HAS_FORK is true)
>>
>> Bert
>>
>> From: Kyle McKay [mailto:mackyle_at_gmail.com]
>> Sent: donderdag 7 mei 2009 15:57
>> To: dev_at_subversion.tigris.org
>> Subject: Re: svn commit: r37615 - trunk/subversion/libsvn_ra_svn
>>
>> On May 7, 2009, at 00:20, Bert Huijben <rhuijben_at_sharpsvn.net> wrote:
>>
>> +static apr_status_t detach_child_cleanup(void *data)
>> +{
>> +#if APR_HAVE_STDLIB_H
>> +#if APR_HAVE_UNISTD_H
>> +#if APR_HAS_FORK
>>
>> I don't think we need the two outer #if's. (We only need the check
>> for
>> fork()).
>>
>>
>> The exit function is declared in stdlib.h. It's not appropriate to
>> call it if we haven't included the header where it's defined (hence
>> the test for APR_HAVE_STDLIB_H).
>>
>> The fork function is declared in unistd.h, it's not appropriate to
>> try and call it if we haven't included the header where it's
>> defined (hence the test for APR_HAVE_UNISTD_H).
>>
>> For this patch it's also not appropriate to call fork if APR isn't
>> using it (hence the test for APR_HAS_FORK).
>>
>> So the patch code is only compiled in if you have an stdlib.h (for
>> exit()) and a unistd.h (for fork()) AND APR is using fork.
>>
>> Are you certain that there aren't some APR-supported and Subversion
>> supported platforms where APR_HAS_FORK is true but one or the other
>> of the standard headers that declare fork/exit are missing? Some
>> older systems perhaps...
>>
>> These defines check whether a specific header file exist and not if
>> functionality exist.
>>
>>
>> Actually the tests are there to avoid calling functions for which
>> we haven't included the header file which declares them.
>>
>> Using these checks is most likely a dumb guess on whether this is
>> running on
>> a Unix system. (stdlib.h is available on any operating system I
>> know and
>> probably part of recent C standards).
>>
>>
>> Bert
>>
>>
>> If stdlib.h is available everywhere, then why does apr define a
>> test for it rather than just always use it?
>>
>> Since apr does define a test for APR_HAVE_STDLIB_H, that suggests
>> apr supports systems without it and that the patch code should too.
>>
>> Kyle
>>
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2096787
Received on 2009-05-07 17:49:45 CEST