Hi David,

On Fri, May 29, 2009 at 09:38:19AM +0100, David Lee wrote:
> On Thu, 28 May 2009, Dejan Muhamedagic wrote:
> 
> > On Thu, May 28, 2009 at 08:33:15PM +0200, Raoul Bhatia [IPAX] wrote:
> >> [...]
> >> please find my current version attached.
> 
> Just a couple more comments, relating to portability.
> 
> >> ____________________________________________________________________
> >
> >> #!/bin/sh
> >> [...]
> 
> The script later contains some features that are specific to "bash".
> So that header should instead read:  "#!/bin/bash".
> 
> >> ##########################################################################
> >>
> >> # Initialization:
> >>
> >> . ${OCF_ROOT}/resource.d/heartbeat/.ocf-shellfuncs
> >>
> >> : ${OCF_RESKEY_binary="/usr/sbin/postfix"}
> 
> Some UN*X systems might have 'postfix' at other locations.  So the 
> "/usr/sbin/postfix" should be a shell variable.

You're right in general, but I think not so in this particular
case. postfix is out of scope of our project and this path is
just a guess (though probably good for the vast majority). Also,
it is only the default, so people having different installations
may set the binary attribute.

> In general the technique is to use 'configure' to determine the pathname 
> and set it in a variable, then write lines such as the above using that 
> variable.  Basically, whenever (as a linux-ha developers) we find 
> ourselves typing a pathname, we should use the configure method to feed it 
> through a shell variable.
> 
> (That fixed pathname and a couple of others are also used later in the 
> script; all such instances ought to use configure-derived variables.)
> 
> >> running() {
> >>     queue=$(postconf $OPTION_CONFIG_DIR -h queue_directory 2>/dev/null || 
> >> echo /var/spool/postfix)
> >>     if [ -f ${queue}/pid/master.pid ]; then
> >>         pid=$(sed 's/ //g' ${queue}/pid/master.pid)
> >>         # what directory does the executable live in.  stupid prelink 
> >> systems.
> >>         dir=$(ls -l /proc/$pid/exe 2>/dev/null | sed 's/.* -> //; 
> >> s/\/[^\/]*$//')
> >>         if [ "X$dir" = "X/usr/lib/postfix" ]; then
> >> [...]
> 
> The above contains two non-Bourne features of 'bash' (e.g. "queue=$(...)") 
> hence my first comment about changing to "#!/bin/bash".

OK. Didn't know that Bourne shell doesn't support $(...). I think
I may have that in some scripts too. In this case it's easy to
replace it with `...`.

> The "/proc/$pid/exe" is Linux-specific.  (But if a script is known to be 
> Linux-sepcific (i.e. script itself will not be valid on (say) *BSD, 
> Darwin, Solaris, etc.) then such things are probably OK.)
> 
> The "/usr/lib/postfix" should come through configure-derived variables.

Right. But those parts are removed anyway.

> But don't let that discourage you!  All the best.

Cheers,

Dejan

> 
> 
> -- 
> 
> :  David Lee                                I.T. Service          :
> :  Senior Systems Programmer                Computer Centre       :
> :  UNIX Team Leader                         Durham University     :
> :                                           South Road            :
> :  http://www.dur.ac.uk/t.d.lee/            Durham DH1 3LE        :
> :  Phone: +44 191 334 2752                  U.K.                  :
> _______________________________________________________
> Linux-HA-Dev: Linux-HA-Dev@lists.linux-ha.org
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev@lists.linux-ha.org
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Reply via email to