[Gelöst] function() Script vereinfachen

Vom einfachen Programm zum fertigen Debian-Paket, Fragen rund um Programmiersprachen, Scripting und Lizenzierung.
Antworten
pixelpirat
Beiträge: 158
Registriert: 05.07.2007 17:22:21

[Gelöst] function() Script vereinfachen

Beitrag von pixelpirat » 10.03.2021 15:19:33

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!
Zuletzt geändert von pixelpirat am 12.03.2021 08:36:27, insgesamt 1-mal geändert.

Benutzeravatar
Livingston
Beiträge: 1364
Registriert: 04.02.2007 22:52:25
Lizenz eigener Beiträge: MIT Lizenz
Wohnort: 127.0.0.1

Re: function() Script vereinfachen

Beitrag von Livingston » 10.03.2021 18:07:17

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.
Der Hauptunterschied zwischen etwas, was möglicherweise kaputtgehen könnte und etwas, was unmöglich kaputtgehen kann, besteht darin, dass sich bei allem, was unmöglich kaputtgehen kann, falls es doch kaputtgeht, normalerweise herausstellt, dass es unmöglich zerlegt oder repariert werden kann.
Douglas Adams

Benutzeravatar
Meillo
Moderator
Beiträge: 8782
Registriert: 21.06.2005 14:55:06
Wohnort: Balmora
Kontaktdaten:

Re: function() Script vereinfachen

Beitrag von Meillo » 10.03.2021 21:58:08

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.)
Use ed once in a while!

pixelpirat
Beiträge: 158
Registriert: 05.07.2007 17:22:21

Re: function() Script vereinfachen

Beitrag von pixelpirat » 11.03.2021 09:43:29

Livingston hat geschrieben: ↑ zum Beitrag ↑
10.03.2021 18:07:17
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.

Benutzeravatar
Livingston
Beiträge: 1364
Registriert: 04.02.2007 22:52:25
Lizenz eigener Beiträge: MIT Lizenz
Wohnort: 127.0.0.1

Re: function() Script vereinfachen

Beitrag von Livingston » 11.03.2021 12:56:48

Jo, jetzt sehe ich es auch. Hatte ich wohl Matsch vor den Augen :mrgreen:
Der Hauptunterschied zwischen etwas, was möglicherweise kaputtgehen könnte und etwas, was unmöglich kaputtgehen kann, besteht darin, dass sich bei allem, was unmöglich kaputtgehen kann, falls es doch kaputtgeht, normalerweise herausstellt, dass es unmöglich zerlegt oder repariert werden kann.
Douglas Adams

Antworten