Eric Blake wrote:
On 05/05/2010 12:54 PM, Jim Meyering wrote:
+ if (!defined $device) { + $device = $self->config("host_block_devices/[$devindex]", undef); + }
This can be equivalently (idiomatically) written as:
$device ||= $self->config("host_block_devices/[$devindex]", undef);
Thanks for the tip.
+ if ($size) { + sysopen(BLK, "$device", O_RDONLY) or return undef; + return undef unless sysseek(BLK, 0, SEEK_END) == $size * 1024; + close(BLK); + }
(Note no need for double quotes around $device; Leaving parens off of some built-ins like "close" is a personal preference (less syntax is better), but obviously keeping in sync with the prevailing style is more important)
This is similar, but doesn't leave BLK open upon failure or size mismatch:
Aargh. I noticed that too, and even have it in my editor, but I hadn't hit save before I did 'git commit'. But your alternative
if ($size) { my $match = (sysopen (BLK, $device, O_RDONLY) && sysseek (BLK, 0, SEEK_END) == $size * 1024); close BLK; $match or return undef; }
is even shorter that what 'git diff' says was still in my editor:
Your "$blocks" is better than "$size", but how about $kb_blocks instead "blocks", to distinguish from the relatively common connotation of 512-byte blocks.
diff --git i/lib/Sys/Virt/TCK.pm w/lib/Sys/Virt/TCK.pm index fc325a3..1fcfdf0 100644 --- i/lib/Sys/Virt/TCK.pm +++ w/lib/Sys/Virt/TCK.pm @@ -835,15 +835,16 @@ sub get_host_block_device { my $devindex = @_ ? shift : 0;
my $device = $self->config("host_block_devices/[$devindex]/path", undef); - my $size = $self->config("host_block_devices/[$devindex]/size", 0); + my $blocks = $self->config("host_block_devices/[$devindex]/size", 0);