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: r816061 - in /httpd/mod_ftp/trunk/modules/ftp: ftp_commands.c ftp_util.c
Date Thu, 17 Sep 2009 06:55:03 GMT
On 17.09.2009 08:10, William A. Rowe, Jr. wrote:
> rjung@apache.org wrote:
>> Author: rjung
>> Date: Thu Sep 17 05:45:18 2009
>> New Revision: 816061
>>
>> URL: http://svn.apache.org/viewvc?rev=816061&view=rev
>> Log:
>> Fix
>> - implicit declaration of function 'fchmod'
>> - 'S_I[RWX](USR|GRP|OTH)' undeclared
> 
> Good catch, but...
> 
>> +#ifdef HAVE_FCHMOD
>> +#ifdef HAVE_SYS_STAT_H
>> +#include <sys/stat.h>
>> +#endif
>> +#endif
> 
> Grrr... this should *never* occur.  The HAVE_tests should not be
> layered.  What is your question here?  (And this goes for your
> commit to ftp_util.c as well.)  Is it FCHMOD or STAT_H?  Is it WIN32
> or STAT_H?
> 
> Including a system header should be harmless, so you should be
> dropping the overarching conditions.

Aha, I see: you moved those includes in r782531 from those files into
ftp_internal.h:

http://svn.apache.org/viewvc?diff_format=h&view=rev&revision=782531

Now in ftp_internal.h the feature APR_HAVE_SYS_STAT_H is tested. And it
seems that in contrast to APR_HAVE_SYS_TYPES_H, the define
APR_HAVE_SYS_STAT_H never makes it into the apr header files. It's
missing in apr.h.in.

So I would say we need to fix APR_HAVE_SYS_STAT_H definition in apr and
should use HAVE_SYS_STAT_H in ftp_internal.c instead.

I will revert the include changes to the two C files.

Anything I overlooked?

Regards,

Rainer

Mime
View raw message