[Gelöst] function() Script vereinfachen

[pixelpirat]

Hallo,

ich habe eine Funktion für den Zustand von Laufwerken geschrieben.
Es soll verschieden Zustände zurück melden und es funktioniert.
Jedoch kommt es mir so extrem if verschachtelt vor, sdoaß ich es noch vereinfachen möchte und bitte um ein Feedback..

Code: Alles auswählen

## Funktion die angeschlossen Platten auf Vorhandensein und Verschlüsselung prüft
#
# Ist die externe Platte verschlüsselt und ist eingebunden?

function check_status()
{
    # Setzt den wert auf das Device.
    # 5 Zustände + Error
    # -- nodevice wenn das Laufwerk mit dem Label nicht gefunden wurde
    # -- true     = Laufwerk vorhanden
    # -- plain    = LW unverschlüsselt
    # -- encrypt  = LW verschlüsselt und geschlossen
    # -- decrypt  = LW entschlüsselt, nicht eingebunden
    # -- ready    " LW eingebunden und bereit für die Verwendung

    [ "$source_debug" = "true" ] && echo -e "\n\nAufgerufene Funktion ${FUNCNAME[0]}";

    drive_label="$1"
    drive_mount="$2"
    local drive_tmp=$(lsblk -ln -po name,label,mountpoint | grep $drive_label)
    if [[ "$drive_tmp" == *"$drive_label"* ]]; then

        if [[ "$drive_tmp"  == *"$drive_mount"* ]]; then

            if [[ $drive_tmp == *"/dev/mapper/"* ]]; then
                drive_status="ready"

            elif [[ $drive_tmp == "/dev/sd"* ]]; then
                drive_status="ready"

            else
                drive_status="error"
                echo -e "Fehler aufgetreten! Unbekanntes Gerät. Abbruch!"
                exit

            fi

        else
            if [[ $drive_tmp == *"/dev/mapper/"* ]]; then
                drive_status="decrypt"

            elif [[ $drive_tmp == "/dev/sd"* ]]; then
                local drive_tmp=($(lsblk -r -po name,label | grep $drive_label))
                [[ $(cryptsetup isLuks $drive_tmp) -eq 0 ]] && drive_status="encrypt" || drive_status="plain"

            else
                drive_status="error"
                echo -e "Fehler aufgetreten! Unbekanntes Gerät. Abbruch!"
                exit

            fi

        fi

    else
         drive_status="nodevice"

    fi


}

Danke im voraus!

[Livingston]

Ich finde es eigentlich recht übersichtlich, aber das ist vielleicht Geschmackssache.
Du kannst hier if...elif-Alternativen ersetzen durch case-Fallunterscheidungen.
Aber tut der Code eigentlich jetzt, was Du willst? Z.B. hier:

Code: Alles auswählen

if [[ "$drive_tmp" == *"$drive_label"* ]]; then

        if [[ "$drive_tmp"  == *"$drive_mount"* ]]; then

            if [[ $drive_tmp == *"/dev/mapper/"* ]]; then
                drive_status="ready"

Kann denn $drive_tmp gleichzeitig $drive_label, $drive_mount und /dev/mapper beinhalten? Wenn nicht, kann das 3. if nie erreicht werden - wahrscheinlich noch nicht mal das 2.

[Meillo]

Hier mein Vorschlag einer Umarbeitung:

Code: Alles auswählen

## Funktion die angeschlossen Platten auf Vorhandensein und Verschlüsselung prüft
#
# Ist die externe Platte verschlüsselt und ist eingebunden?

function check_status()
{
    # Setzt den wert auf das Device.
    # 5 Zustände + Error
    # -- nodevice wenn das Laufwerk mit dem Label nicht gefunden wurde
    # -- true     = Laufwerk vorhanden
    # -- plain    = LW unverschlüsselt
    # -- encrypt  = LW verschlüsselt und geschlossen
    # -- decrypt  = LW entschlüsselt, nicht eingebunden
    # -- ready    " LW eingebunden und bereit für die Verwendung

    [ "$source_debug" = "true" ] && echo -e "\n\nAufgerufene Funktion ${FUNCNAME[0]}

    drive_label="$1"
    drive_mount="$2"
    local drive_tmp=$(lsblk -ln -po name,label,mountpoint | grep $drive_label)

    if [[ "$drive_tmp" == *"$drive_label"* ]]; then
         drive_status="nodevice"
         return
    fi

    if [[ $drive_tmp == *"/dev/mapper/"* ]]; then
        if [[ "$drive_tmp"  == *"$drive_mount"* ]]; then
            drive_status="ready"
        else
            drive_status="decrypt"
        fi

    elif [[ $drive_tmp == "/dev/sd"* ]]; then
        if [[ "$drive_tmp"  == *"$drive_mount"* ]]; then
            drive_status="ready"
        else
            local drive_tmp=($(lsblk -r -po name,label | grep $drive_label))
            [[ $(cryptsetup isLuks $drive_tmp) -eq 0 ]] && drive_status="encrypt
        fi

    else
        drive_status="error"
        echo -e "Fehler aufgetreten! Unbekanntes Gerät. Abbruch!"
        exit
    fi
}

Was ich geaendert habe:

- Den nodevice-Fall gleich aussortieren statt den Gutfall einzuschachteln.

- Die Verschachtelung der ifs umgedreht, weil so der Fehlerfall nur noch einmal da ist, und weil es einfacher zu lesen ist, wenn das mehrfache if aussen ist und das nur if-else innen.

So finde ich es schon lesbarer. Wenn dir das noch nicht reicht, solltest du Teile des Codes in Hilfsfunktionen auslagern.

(Auf den inhaltlichen Aspekt der Logik habe ich nicht geachtet. Ich habe nur die Verschachtelung umgebaut.)

[pixelpirat]

Ich finde es eigentlich recht übersichtlich, aber das ist vielleicht Geschmackssache.
Du kannst hier if...elif-Alternativen ersetzen durch case-Fallunterscheidungen.
Aber tut der Code eigentlich jetzt, was Du willst? Z.B. hier:

Code: Alles auswählen

if [[ "$drive_tmp" == *"$drive_label"* ]]; then

        if [[ "$drive_tmp"  == *"$drive_mount"* ]]; then

            if [[ $drive_tmp == *"/dev/mapper/"* ]]; then
                drive_status="ready"

Kann denn $drive_tmp gleichzeitig $drive_label, $drive_mount und /dev/mapper beinhalten? Wenn nicht, kann das 3. if nie erreicht werden - wahrscheinlich noch nicht mal das 2.

Ja, das kann erreicht werden.

$drive_mount enthält den Mountpoint wo, das LW eingehangen werden soll.
$drive_label enthält das Label des Lauwerkes
$drive_tmp kann verschiedene Devices mit dem selben Label enthalten.
Hier ein Beispiel mit einem Label = Secure und geöffneten Luks Container

Code: Alles auswählen

echo $(lsblk -r -po name,label | grep Secure)
/dev/sdd2 Secure /dev/mapper/Secure Secure

vor dem Luksopen bekomme ich diese Ausgabe

Code: Alles auswählen

echo $(lsblk -r -po name,label | grep Secure)
/dev/sdd2 Secure 

Ist die Platte bereits gemounted dann habe ich noch den Mountpoint in dem String drin.

[Livingston]

Jo, jetzt sehe ich es auch. Hatte ich wohl Matsch vor den Augen