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.