You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Guenter Knauf <fu...@apache.org> on 2011/03/16 04:23:21 UTC

initgroups() in mpm_common.c

Hi all,
I would like to get rid of a missing prototype compiler warning ...
in mpm_common.c we provide an initgroups() for platforms which dont have 
that function. This function just returns 0 for _OSD_POSIX, OS2, WIN32 
and NETWARE. Currently in httpd sources there are only 2 places where 
initgroups() is used: mod_unixd.c and suexec.c; both will not be 
compiled for at least WIN32 and NETWARE, and OS2 does #ifdef the 
initgroups() calls in mod_unixd.c - so only platform where the fake stub 
might be used is _OSD_POSIX (BS2000?).

One possible fix would be to avoid compiling the stub at all:
Index: server/mpm_common.c
===================================================================
--- server/mpm_common.c	(revision 1082026)
+++ server/mpm_common.c	(working copy)
@@ -192,10 +192,11 @@
  }
  #endif

+#if !defined(OS2) && !defined(WIN32) && !defined(NETWARE)
  #ifndef HAVE_INITGROUPS
  int initgroups(const char *name, gid_t basegid)
  {
-#if defined(_OSD_POSIX) || defined(OS2) || defined(WIN32) || 
defined(NETWARE)
+#if defined(_OSD_POSIX)
      return 0;
  #else
      gid_t groups[NGROUPS_MAX];
@@ -222,7 +223,8 @@
      return setgroups(index, groups);
  #endif
  }
-#endif /* def NEED_INITGROUPS */
+#endif /* def HAVE_INITGROUPS */
+#endif

  /* standard mpm configuration handling */
  const char *ap_pid_fname = NULL;

another one would be to add a prototype to mpm_common.h ...

any preferences?

Gün.



RE: initgroups() in mpm_common.c

Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
 

> -----Original Message-----
> From: Guenter Knauf 
> Sent: Mittwoch, 16. März 2011 04:23
> To: httpd Developer List
> Subject: initgroups() in mpm_common.c
> 
> Hi all,
> I would like to get rid of a missing prototype compiler warning ...
> in mpm_common.c we provide an initgroups() for platforms 
> which dont have 
> that function. This function just returns 0 for _OSD_POSIX, 
> OS2, WIN32 
> and NETWARE. Currently in httpd sources there are only 2 places where 
> initgroups() is used: mod_unixd.c and suexec.c; both will not be 
> compiled for at least WIN32 and NETWARE, and OS2 does #ifdef the 
> initgroups() calls in mod_unixd.c - so only platform where 
> the fake stub 
> might be used is _OSD_POSIX (BS2000?).
> 
> One possible fix would be to avoid compiling the stub at all:
> Index: server/mpm_common.c
> ===================================================================
> --- server/mpm_common.c	(revision 1082026)
> +++ server/mpm_common.c	(working copy)
> @@ -192,10 +192,11 @@
>   }
>   #endif
> 
> +#if !defined(OS2) && !defined(WIN32) && !defined(NETWARE)
>   #ifndef HAVE_INITGROUPS
>   int initgroups(const char *name, gid_t basegid)
>   {
> -#if defined(_OSD_POSIX) || defined(OS2) || defined(WIN32) || 
> defined(NETWARE)
> +#if defined(_OSD_POSIX)
>       return 0;
>   #else
>       gid_t groups[NGROUPS_MAX];
> @@ -222,7 +223,8 @@
>       return setgroups(index, groups);
>   #endif
>   }
> -#endif /* def NEED_INITGROUPS */
> +#endif /* def HAVE_INITGROUPS */
> +#endif
> 
>   /* standard mpm configuration handling */
>   const char *ap_pid_fname = NULL;
> 
> another one would be to add a prototype to mpm_common.h ...

I guess we should do that at least for the platforms where we provide the stub
(e.g. _OSD_POSIX). So I am not opposed to the patch above, but I think we should
have a prototype for the platforms where we create the stub.

Regards

Rüdiger

Re: initgroups() in mpm_common.c

Posted by Guenter Knauf <fu...@apache.org>.
Am 16.03.2011 13:04, schrieb Dan Poirier:
> The function is already wrapped in #ifndef HAVE_INITGROUPS, shouldn't
> that prevent it being compiled on platforms that don't need it?  If not,
> that should probably be fixed (or else I misunderstand what's
> happening).
no - cant be fixed. For whatever reason the idea is to have the function 
on all platforms - (but as I pointed out the fake stub part is not used 
by any other platform than probably BS2000):
#ifndef HAVE_INITGROUPS
int initgroups(const char *name, gid_t basegid)
{
#if defined(_OSD_POSIX) || defined(OS2) || defined(WIN32) || 
defined(NETWARE)
     return 0;
#else
....
#endif

> Also, I'm opposed to adding more #if PLATFORM_XYZ unless we absolutely
> have to.
agreed.
>
> Anyway, is there some reason why we wouldn't just add the prototype?
no, I'm fine with that too. I've added one in mpm_common.h since then it 
makes sense and avoids compiler warnings too for those platforms which 
compile mod_unixd.c and dont have HAVE_INITGROUPS defined:
http://svn.apache.org/viewvc?rev=1082250&view=rev

Gün.



Re: initgroups() in mpm_common.c

Posted by Dan Poirier <po...@pobox.com>.
On Tue. 2011-03-15 at 11:23 PM EDT, Guenter Knauf <fu...@apache.org> wrote:

> Hi all,
> I would like to get rid of a missing prototype compiler warning ...
> in mpm_common.c we provide an initgroups() for platforms which dont have 
> that function. This function just returns 0 for _OSD_POSIX, OS2, WIN32 
> and NETWARE. Currently in httpd sources there are only 2 places where 
> initgroups() is used: mod_unixd.c and suexec.c; both will not be 
> compiled for at least WIN32 and NETWARE, and OS2 does #ifdef the 
> initgroups() calls in mod_unixd.c - so only platform where the fake stub 
> might be used is _OSD_POSIX (BS2000?).
>
> One possible fix would be to avoid compiling the stub at all:
> Index: server/mpm_common.c
> ===================================================================
> --- server/mpm_common.c	(revision 1082026)
> +++ server/mpm_common.c	(working copy)
> @@ -192,10 +192,11 @@
>   }
>   #endif
>
> +#if !defined(OS2) && !defined(WIN32) && !defined(NETWARE)
>   #ifndef HAVE_INITGROUPS
>   int initgroups(const char *name, gid_t basegid)
>   {
> -#if defined(_OSD_POSIX) || defined(OS2) || defined(WIN32) || 
> defined(NETWARE)
> +#if defined(_OSD_POSIX)
>       return 0;
>   #else
>       gid_t groups[NGROUPS_MAX];
> @@ -222,7 +223,8 @@
>       return setgroups(index, groups);
>   #endif
>   }
> -#endif /* def NEED_INITGROUPS */
> +#endif /* def HAVE_INITGROUPS */
> +#endif
>
>   /* standard mpm configuration handling */
>   const char *ap_pid_fname = NULL;
>
> another one would be to add a prototype to mpm_common.h ...
>
> any preferences?
>
> Gün.

The function is already wrapped in #ifndef HAVE_INITGROUPS, shouldn't
that prevent it being compiled on platforms that don't need it?  If not,
that should probably be fixed (or else I misunderstand what's
happening).

Also, I'm opposed to adding more #if PLATFORM_XYZ unless we absolutely
have to.

Anyway, is there some reason why we wouldn't just add the prototype?

Dan