Code review comment for lp:~jderose/ecryptfs/fix-1597154

Tyler,

Okay, resubmitting with your two issues addressed:

1) I didn't come up with a unified sed expression I liked for building $drive, so I'm still using a conditional, but this time with:

  grep -qE "^/dev/.+[0-9]+p[0-9]+$"

This means the code now correctly handles the generic /dev/foo0p1 pattern. I agree that this is a better approach, although I still feel there is a certain fragility here as we're still making some assumption that we don't know for sure will hold for future types of block devices that merely match this naming pattern.

2) Nice catch! I didn't notice that the original code didn't correctly handle multi-digit partition numbers. In all cases, I'm now building $partno with:

  partno=$(echo $swap | sed "s/.\+[^0-9]\([0-9]\+\)/\1/")

I also updated my (out-of-band) test script, which now correctly handles all the key corner cases for existing types of block devices that I can think of:

"""
#!/bin/sh -e

test_fix() {
    swap=$1
    if echo $swap | grep -qE "^/dev/.+[0-9]+p[0-9]+$"; then
        drive=$(echo $swap | sed "s/\(.\+[0-9]\)p[0-9]\+/\1/")
    else
        drive=$(echo $swap | sed "s/\(.\+[^0-9]\)[0-9]\+/\1/")
    fi
    partno=$(echo $swap | sed "s/.\+[^0-9]\([0-9]\+\)/\1/")
    echo "$swap\t$drive\t$partno"
}

test_fix "/dev/sda3"
test_fix "/dev/sda33"
test_fix "/dev/sdp3"
test_fix "/dev/sdp33"

echo ""
test_fix "/dev/vda3"
test_fix "/dev/vda33"
test_fix "/dev/vdp3"
test_fix "/dev/vdp33"

echo ""
test_fix "/dev/mmcblk0p3"
test_fix "/dev/mmcblk0p33"
test_fix "/dev/mmcblk99p3"
test_fix "/dev/mmcblk99p33"

echo ""
test_fix "/dev/nvme0n1p3"
test_fix "/dev/nvme0n1p33"
test_fix "/dev/nvme99n1p3"
test_fix "/dev/nvme99n1p33"

echo ""
test_fix "/dev/nvme0n11p3"
test_fix "/dev/nvme0n11p33"
test_fix "/dev/nvme99n11p3"
test_fix "/dev/nvme99n11p33"
"""

Please let me know if I missed anything or if you'd like further tweaks!