httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rainer Jung <rainer.j...@kippdata.de>
Subject Re: svn commit: r816074 - in /httpd/mod_ftp/trunk/modules/ftp: ftp_commands.c ftp_internal.h ftp_util.c
Date Thu, 17 Sep 2009 09:03:35 GMT
On 17.09.2009 10:18, William A. Rowe, Jr. wrote:
> rjung@apache.org wrote:
>>
>> Modified: httpd/mod_ftp/trunk/modules/ftp/ftp_internal.h
>> URL: http://svn.apache.org/viewvc/httpd/mod_ftp/trunk/modules/ftp/ftp_internal.h?rev=816074&r1=816073&r2=816074&view=diff
>> ==============================================================================
>> --- httpd/mod_ftp/trunk/modules/ftp/ftp_internal.h (original)
>> +++ httpd/mod_ftp/trunk/modules/ftp/ftp_internal.h Thu Sep 17 07:00:24 2009
>> @@ -84,6 +84,8 @@
>>  
>>  #if APR_HAVE_SYS_STAT_H
>>  #include <sys/stat.h>
>> +#elif HAVE_SYS_STAT_H
>> +#include <sys/stat.h>
>>  #endif
> 
> NAK.  The fragment above makes zero sense, please revert.
> 
> #if APR_HAVE_SYS_STAT_H
> 
> should be sufficent.  If apr does not provide it consistently, and httpd
> has, then the test becomes
> 
> #ifdef HAVE_SYS_STAT_H
> 
> which is an altogether different beast.

Sorry, of course.

> First clue that the code above was
> wrong is that you included the same code for both cases.

This I don't understand. Which cases?

> So provided that
> you had no clue if APR consistently provided this and you wanted to rely

I did have a clue (at least I thought so ;) ), I posted to dev@apr
separately. It does *not* provide it.

> upon config.h, then it becomes
> 
> #if (defined(APR_HAVE_SYS_STAT_H) && APR_HAVE_SYS_STAT_H) \
>     || defined(HAVE_SYS_STAT_H)
> #include...
> 
> but we know we don't need to go that far.

Hmmm? Why doesn't that look like the right thing? You want to use only
HAVE_SYS_STAT_H instead? I thought allowing the APR variant made some
sense to support a forthcoming APR version, that might have the APR_
variant defined.

> I'll answer the question you raised on list tomorrow, although it certainly
> seemed like an apr question and not an httpd question.

Yes, see dev@apr.

Not that after reverting the source doesn't build on Solaris using gcc
4.1. The undefined file permission symbols make it err.

Regards,

Rainer

Mime
View raw message