OT: Fehlerbehandlung und "elegantes" Programmieren

[mikrosaft]

[edit]
Asche auf mein Haupt - ich habe im falschen Bereich gepostet. Um den Post selbst zu löschen und nach "Konsole" zu verschieben ist es leider schon zu spät, da schon eine Antwort da ist.
Sorry...
[/edit]

Hallo,

zuallererst möchte ich anmerken: das Script läuft Problemlos und hier geht es nur um Feinheiten - deshalb OT. Ich würde gerne wissen, ob und wie man die Fehlerbehandlung optimieren könnte, und wie man das ganze vllt. etwas eleganter schreiben könnte.
Hier das Script:

#!/bin/bash


#####Don't change this lines#####
exec 1>> /tmp/scan2fax.log
exec 2>> /tmp/scan2fax.err
trap 'schluss -127' 2 15
# set -x


#####User dependent settings######
#this Variables may be changed through user
PATH="/sbin:/usr/sbin:/usr/local/sbin:/opt/kde3/sbin:/opt/gnome/sbin:/root/bin"
PATH="$PATH:/usr/local/bin:/usr/bin:/usr/X11R6/bin:/bin:/usr/games"
PATH="$PATH:/opt/gnome/bin:/opt/kde3/bin:/usr/lib/mit/bin:/usr/lib/mit/sbin"
PAGE=1
user="user" #set capisuite-user here
export SANE_DEBUG_PIXMA=10 #scanner-dependent
export DISPLAY=:0.0


#####Don't change this lines till end of script#####

function schluss(){
   rm /tmp/.scan2fax.lock
   log2 "scan2fax killed: Exit $1"
   log1 "scan2fax wurde unerwartet beendet! Exit $1"
   exit $1
}

function log1(){
   echo "`date`: $1"
}

function log2(){
   echo "`date`: $1" >&2
}

function check_error(){
   if [ $1 -ne 0 ]
   then
      log2 "Exit $1"
      schluss $1
   fi
}

function check_lock(){
   if [ -e /tmp/.scan2fax.lock ]
   then
      log2 "Vorhandene Instanz entdeckt! Abbruch!"
      check_error "-1"
   else
      echo $$ > /tmp/.scan2fax.lock
      # rm -f /tmp/fax/*
   fi
}

check_scanner_lock(){
   SI=$(pidof scanimage | grep "[1-9]" | wc -l)
   XSI=$(pidof xscanimage | grep "[1-9]" | wc -l)
   XS=$(pidof xsane | grep "[1-9]" | wc -l)
   if [ $SI -eq 0 ] 
   then
      if [ $XSI -eq 0 ]
      then
         if [ $XS -eq 0 ]
         then
            echo 0
         else
            echo -1
         fi
      else 
         echo -1
      fi
   else
      echo -1
   fi
}

waitfor_scanner(){
   while [ $(check_scanner_lock) -ne 0 ]
   do
      log1 "Nonexclusive access 2 Scanner..."
      sleep 2
   done
}

get_button(){
   BUTTON=`scanimage -nv --button-update 2>&1 | \
   grep "00000000:01" | wc -c`
   if [ $BUTTON -ne 0 ]
   then
      echo 1
   else
      echo 0
   fi
}

function print_log(){
   if [ -e /var/spool/capisuite/done/$user-fax-$1.txt ]
   then
      echo 'Result="3400,0 (Success)"' >> /var/spool/capisuite/done/$user-fax-$1.txt
      lp -d tp0 /var/spool/capisuite/done/$user-fax-$1.txt
      log1 "Fax-Protokoll gedruckt"
   elif [ -e /var/spool/capisuite/failed/$user-fax-$1.txt ]
   then
      echo 'Result="3490,3314 (Failed)"' >> /var/spool/capisuite/failed/$user-fax-$1.txt
      lp -d tp0 /var/spool/capisuite/failed/$user-fax-$1.txt
      log1 "Fax-Protokoll gedruckt"
   else
      sleep 10
      print_log $1
   fi
}

#####main#####
# check if there are other instances
check_lock

log1 "Script gestartet"

while :
do
   
   # if nonexclusive access to scanner
   if [ $(check_scanner_lock) -eq 0 ]
   then
      waitfor_scanner
   fi
   
   if [ $(get_button) -ne 0 ]
   then
      if [ $(get_button) -ne 0 ]
      then
         # Button pressed twice
         PAGE=1
         xset dpms force on
         gs -q -dNOPAUSE -dBATCH -sDEVICE=pdfwrite \
         -sOutputFile=/tmp/fax/fax.pdf /tmp/fax/fax_*.ps
         check_error $?
         log1 "fax.pdf erstellt"
         #Dialog
         if kdialog --display :0.0 --yesno "Vorschau anzeigen?"
         then
            acroread /tmp/fax/fax.pdf
         fi
         nummer=`kdialog --display :0.0 --inputbox "Bitte Faxnummer \
         eingeben (ohne Sonderzeichen):" "$nummer"`
         RESULT=$(su -c "capisuitefax -d $nummer /tmp/fax/fax.pdf" $user)
         ERR=$?
         if [ $ERR -eq 0 ]
         then
            log1 "Fax an $nummer versandt"
            log1 "$RESULT"
            NEXT=$(cat /var/spool/capisuite/users/$user/sendq/fax-nextnr)
            let THIS=$NEXT-1
            print_log $THIS
         else
            log2 "Beim versand der Nachricht ist ein Fehler aufgetreten!"
            # log2 "$RESULT"
         fi
         #make clean
         rm -f /tmp/fax/*
      else
         #Button pressed once
         log1 "Scanne Seite $PAGE"
         ( scanimage --resolution 150 | convert - -gamma 0.7 -threshold \
         45000,20000 -monochrome /tmp/fax/fax_$PAGE.pnm ) 2> /dev/null
         check_error $?
         pnmtops /tmp/fax/fax_$PAGE.pnm > /tmp/fax/fax_$PAGE.ps
         log1 "fax_$PAGE.ps erstellt"
         let PAGE=$PAGE+1
         sleep 2
      fi
   fi
done
#####end#####

Kommentare...?

Gruss
ms

[edit]Kritikpunkte von jengelh verbessert[/edit]

 

[jengelh]

#this Variables may be changed through user
PATH=/sbin:/usr/sbin:/usr/local/sbin:/opt/kde3/sbin:/opt/gnome/sbin:/root/bin:/usr/local/bin:/usr/bin:/usr/X11R6/bin:/bin:/usr/games:/opt/gnome/bin:/opt/kde3/bin:/usr/lib/mit/bin:/usr/lib/mit/sbin

Nr. 1: Richtig quoten, also

PATH="/sbin:....."

Nr. 2: Mach dir mehrere Zeilen,

PATH="/sbin:/usr/sbin:...";
PATH="$PATH:/noch/ein/ding:....";

function schluss(){
:
rm /tmp/.scan2fax.lock
log2 "scan2fax killed: Exit $1"
log1 "scan2fax wurde unerwartet beendet! Exit $1"
exit $1
}

Die :-Zeile ist überflüssig.

function check_error(){
if [ $1 != 0 ]
then
log2 "Exit $1"
schluss $1
fi
}

if [ "$1" -ne 0 ]

Manpage zu test(1) lesen, Zahlen soll man per -eq/-ne vergleichen, !=/== ist für Strings.

check_scanner_lock(){
SI=$(pidof scanimage | grep "[1.9]" | wc -l)

[1-9]?

if [ $SI == 0 ]

Nochmal == / -eq

BUTTON=`${PFAD}scanimage -nv --button-update 2>&1 | ${PFAD}grep "00000000:01" | ${PFAD}wc -c`

Was soll ${PFAD}? Ist doch im $PATH.

function print_log(){
if [ -e /var/spool/capisuite/done/${user}-fax-${1}.txt ]

${user} bringt hier nichts, also reicht $user. Gleiches gilt für andere.

while [ 1 ]

while :; do

 

[mikrosaft]

oha, man merkt wohl, dass ich bisher nur "quick'n'dirty"-Scripte zusammengeschustert habe... :roll:

habe nachgebessert...

Danke & Gruss
ms

 

[jengelh]

noch mal

 

[mikrosaft]

noch mal

, dass jedes vorkommen von Variablen gequotet werden muss... Allerdings verstehe ich das nicht

-         let PAGE=$PAGE+1
+         PAGE=$[$PAGE+1]

Wo kann ich mehr darüber lesen? In den HowTos und man-Pages die ich bisher gewälzt habe, wurde immer let benutzt...
Ich bin gerade dabei, das Script zu vereinfachen. Solche Konstrukte z.B.

   SI=$(pidof scanimage | grep "[1-9]" | wc -l)
   XSI=$(pidof xscanimage | grep "[1-9]" | wc -l)
   XS=$(pidof xsane | grep "[1-9]" | wc -l)
   if [ $SI -eq 0 ]
   then
      if [ $XSI -eq 0 ]
      then
         if [ $XS -eq 0 ]
         then
            echo 0
         else
            echo -1
         fi
      else
         echo -1
      fi
   else
      echo -1
   fi

lassen sich ja z.B. duch so etwas ersetzen:

if (pidof xscanimage || pidof scanimage || pidof xsane) 2>&1 > /dev/null
   then echo 1
   else echo 0
fi

Das Script befindet sich also gerade heavy im Umbau... Deinen (nochmaligen) Hinweis auf das Quoten werde ich allerdings berücksichtigen.
[edit]
hier http://www.rafb.net/paste/results/LYic3A61.html findet sich Version 2 vom script
[/edit]

Gruss
ms

PS: Warum ist der Tread so in die Breite gegangen? In den Scripten sind doch mittlerweile Zeilenumbrüche drin... :?