[Libguestfs] [PATCH] Export inspect_linux_kernel in Lib.pm

[Matthew Booth]

On 19/08/09 15:18, Jim Meyering wrote:
> Matthew Booth wrote:
>> diff --git a/perl/lib/Sys/Guestfs/Lib.pm b/perl/lib/Sys/Guestfs/Lib.pm
> ...
>> @@ -1551,10 +1552,19 @@ sub _check_for_kernels
> ...
>>                   # Check the kernel was recognised
>>                   if(defined($kernel)) {
>> +                    # Put this kernel on the top level kernel list
>> +                    my $kernels = $os->{kernels};
>> +                    if(!defined($kernels)) {
>> +                        $kernels = [];
>> +                        $os->{kernels} = $kernels;
>> +                    }
>> +                    push(@$kernels, $kernel);
>> +
>
> Hi Matt,
>
> It took me too long to see what was being done above,
> so I rewrote it in a way that I found easier to read:
>
> How about this?
>
> diff --git a/perl/lib/Sys/Guestfs/Lib.pm b/perl/lib/Sys/Guestfs/Lib.pm
> index dfa79af..2acdec6 100644
> --- a/perl/lib/Sys/Guestfs/Lib.pm
> +++ b/perl/lib/Sys/Guestfs/Lib.pm
> @@ -1558,12 +1558,8 @@ sub _check_for_kernels
>                   # Check the kernel was recognised
>                   if(defined($kernel)) {
>                       # Put this kernel on the top level kernel list
> -                    my $kernels = $os->{kernels};
> -                    if(!defined($kernels)) {
> -                        $kernels = [];
> -                        $os->{kernels} = $kernels;
> -                    }
> -                    push(@$kernels, $kernel);
> +                    $os->{kernels} ||= [];
> +                    push(@{$os->{kernels}}, $kernel);
>
>                       $config{kernel} = $kernel;
>
>
> The ||= operator may look odd, but it's another of these idiom things.
> Once you see it a few times, then "get it", and start using it,
> you'll never go back.
>
> Which would you prefer to read/maintain?  This:
>
>      # Ensure the array-ref is initialized.
>      if (!defined $some_long_name->[$some_complicated_expression]) {
>          $some_long_name->[$some_complicated_expression] = [];
>      }
>
>      # Append to it.
>      ...
>
> or this:
>
>      # Ensure the array-ref is initialized.
>      $some_long_name->[$some_complicated_expression] ||= [];
>
>      # Append to it.
>      ...

I haven't come across that idiom, and I like it. However, I've already 
pushed this patch now and I'm not sure it merits its own revision given 
the number of other places I could make the same change.

I suggest making this change over time as code is touched.

Matt
-- 
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team

M:       +44 (0)7977 267231
GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490