You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by rj...@apache.org on 2009/09/17 09:00:25 UTC
svn commit: r816074 - in /httpd/mod_ftp/trunk/modules/ftp: ftp_commands.c
ftp_internal.h ftp_util.c
Author: rjung
Date: Thu Sep 17 07:00:24 2009
New Revision: 816074
URL: http://svn.apache.org/viewvc?rev=816074&view=rev
Log:
Revert r816061 and instead follow along the lines
of r782531.
APR currentyl doesn't correctly define APR_HAVE_SYS_STAT_H.
So fall back to HAVE_SYS_STAT_H as a second option.
Modified:
httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c
httpd/mod_ftp/trunk/modules/ftp/ftp_internal.h
httpd/mod_ftp/trunk/modules/ftp/ftp_util.c
Modified: httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c
URL: http://svn.apache.org/viewvc/httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c?rev=816074&r1=816073&r2=816074&view=diff
==============================================================================
--- httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c (original)
+++ httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c Thu Sep 17 07:00:24 2009
@@ -32,12 +32,6 @@
#include <limits.h>
#endif
-#ifdef HAVE_FCHMOD
-#ifdef HAVE_SYS_STAT_H
-#include <sys/stat.h>
-#endif
-#endif
-
/* Wish APR had one of these */
#if defined(WIN32) || defined(USE_WINSOCK)
#define FTP_APR_EADDRINUSE (APR_OS_START_SYSERR + WSAEADDRINUSE)
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
/* SSL Filter name */
Modified: httpd/mod_ftp/trunk/modules/ftp/ftp_util.c
URL: http://svn.apache.org/viewvc/httpd/mod_ftp/trunk/modules/ftp/ftp_util.c?rev=816074&r1=816073&r2=816074&view=diff
==============================================================================
--- httpd/mod_ftp/trunk/modules/ftp/ftp_util.c (original)
+++ httpd/mod_ftp/trunk/modules/ftp/ftp_util.c Thu Sep 17 07:00:24 2009
@@ -25,12 +25,6 @@
#include "apr_fnmatch.h"
#include "ap_mpm.h" /* For MPM query interface */
-#ifndef WIN32
-#ifdef HAVE_SYS_STAT_H
-#include <sys/stat.h>
-#endif
-#endif
-
/* Warning; the *arg is consumed, manipulated and must have the same lifetime
* as the desired *addr results!
*/
Re: svn commit: r816074 - in /httpd/mod_ftp/trunk/modules/ftp: ftp_commands.c
ftp_internal.h ftp_util.c
Posted by Rainer Jung <ra...@kippdata.de>.
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
Re: svn commit: r816074 - in /httpd/mod_ftp/trunk/modules/ftp: ftp_commands.c ftp_internal.h ftp_util.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Sep 17, 2009, at 4:18 AM, 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. First clue that the code
> above was
> wrong is that you included the same code for both cases. So
> provided that
> you had no clue if APR consistently provided this and you wanted to
> rely
> 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.
>
Grok ./modules/generators/mod_cgid.c
/* ### should be tossed in favor of APR */
#include <sys/stat.h>
btw :)
I'm sure there's some history there...
Re: svn commit: r816074 - in /httpd/mod_ftp/trunk/modules/ftp: ftp_commands.c
ftp_internal.h ftp_util.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
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. First clue that the code above was
wrong is that you included the same code for both cases. So provided that
you had no clue if APR consistently provided this and you wanted to rely
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.
I'll answer the question you raised on list tomorrow, although it certainly
seemed like an apr question and not an httpd question.