Re: [PATCH 1/2] git-instaweb: support Fedora/Red Hat apache module path

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Sebastian Kisela <skisela@xxxxxxxxxx> writes:

> On Fedora-derived systems, the apache httpd package installs modules
> under /usr/lib{,64}/httpd/modules, depending on whether the system is
> 32- or 64-bit.  A symlink from /etc/httpd/modules is created which
> points to the proper module path.  Use it to support apache on Fedora,
> CentOS, and Red Hat systems.
>
> Written with assistance of Todd Zullinger <tmz@xxxxxxxxx>
>
> Signed-off-by: Sebastian Kisela <skisela@xxxxxxxxxx>
> ---
>  git-instaweb.sh | 2 ++
>  1 file changed, 2 insertions(+)

Thanks for a patch, and welcome to the Git development community.

> diff --git a/git-instaweb.sh b/git-instaweb.sh
> index 47e38f34c..e42e58528 100755
> --- a/git-instaweb.sh
> +++ b/git-instaweb.sh
> @@ -332,6 +332,8 @@ apache2_conf () {
>  			module_path="/usr/lib/httpd/modules"
>  		test -d "/usr/lib/apache2/modules" &&
>  			module_path="/usr/lib/apache2/modules"
> +		test -d "/etc/httpd/modules" &&
> +			module_path="/etc/httpd/modules"

The original already has the same issue with two entries but this
patch makes it even worse by adding yet another one.  The longer the
code follows a bad pattern, the more it encourages future developers
to follow the same bad pattern, and usually the best time to do a
clean up like the following

	if test -z "$module_path"
	then
		for candidate in \
			/etc/httpd \
			/usr/lib/apache2 \
			/usr/lib/httpd \
		do
			if test -d "$candidate/modules"
			then
				module_path="$candidate/modules"
				break
			fi
		done
	fi

is when you go from 2 to 3.  Two points to note are:

 - It would be easier to add the fourth one this way

 - The explicit "break" makes it clear that the paths are listed in
   decreasing order of precedence (i.e. /etc/httpd if exists makes
   /usr/lib/httpd ignored even if the latter exists); the original
   "test -d X && M=X ; test -d Y && M=Y" gives higher precedence to
   the later items but readers need to wonder if it is intended or
   the code is simply being sloppy.

Hope this helps.






[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux