[libvirt] [PATCH] ruby-libvirt: Don't crash in leases_wrap() by passing NULLs to rb_str_new2()

[Dan Williams]

On Thu, 2016-01-14 at 13:19 -0500, Laine Stump wrote:
> On 01/14/2016 11:01 AM, Dan Williams wrote:
> > On Thu, 2016-01-07 at 11:12 -0600, Dan Williams wrote:
> > > Not all lease values are mandatory, and when they aren't supplied
> > > by the libvirt driver they get set to NULL.  That makes
> > > rb_str_new2() bail out.
> > Ping?  Does this patch look OK or is there anything else I need to
> > do
> > with it?  Is the submission procure for ruby-libvirt different than
> > normal libvirt?
> 
> As far as I can see, posting to libvir-list is the correct thing for 
> ruby-libvirt patches, I think it's just that very few people use it,
> so 
> most of us don't feel comfortable ACKing anything for it.

Thanks!  I'd bet a lot more people use it than you think, since it's a
dependency of vagrant-libvirt by way of fog.  So you can't stand up a
vagrant machine using libvirt without it...

Dan

> It looks like Chris Lalancette is the maintainer and has been the
> author 
> of nearly every patch in the last couple years, so I'm Cc'ing him to
> be 
> sure it see it.
> 
> 
> > 
> > Thanks!
> > Dan
> > 
> > > Signed-off-by: Dan Williams <dcbw at redhat.com>
> > > ---
> > > For example using the qemu driver we don't get 'iaid', and that
> > > makes ruby unhappy:
> > > 
> > > [{"iface"=>"virbr1", "expirytime"=>1452189569, "type"=>0,
> > > "mac"=>"52:54:00:05:2b:6f",
> > > "ipaddr"=>"192.168.121.49", "prefix"=>24,
> > > "hostname"=>"openshiftdev",
> > > "clientid"=>"ff:00:05:2b:6f:00:01:00:01:1e:21:55:ed:52:54:00:05:2
> > > b:6f
> > > "}]
> > > 
> > >   ext/libvirt/network.c | 18 ++++++++++++------
> > >   1 file changed, 12 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/ext/libvirt/network.c b/ext/libvirt/network.c
> > > index 7c77d73..c250d7d 100644
> > > --- a/ext/libvirt/network.c
> > > +++ b/ext/libvirt/network.c
> > > @@ -269,14 +269,20 @@ static VALUE leases_wrap(VALUE arg)
> > >           rb_hash_aset(hash, rb_str_new2("expirytime"),
> > >                        LL2NUM(lease->expirytime));
> > >           rb_hash_aset(hash, rb_str_new2("type"), INT2NUM(lease
> > > ->type));
> > > -        rb_hash_aset(hash, rb_str_new2("mac"), rb_str_new2(lease
> > > ->mac));
> > > -        rb_hash_aset(hash, rb_str_new2("iaid"),
> > > rb_str_new2(lease
> > > ->iaid));
> > > +        if (lease->mac)
> > > +            rb_hash_aset(hash, rb_str_new2("mac"),
> > > rb_str_new2(lease
> > > ->mac));
> > > +        if (lease->iaid)
> > > +            rb_hash_aset(hash, rb_str_new2("iaid"),
> > > rb_str_new2(lease->iaid));
> > >           rb_hash_aset(hash, rb_str_new2("ipaddr"),
> > > rb_str_new2(lease
> > > ->ipaddr));
> > >           rb_hash_aset(hash, rb_str_new2("prefix"),
> > > UINT2NUM(lease
> > > ->prefix));
> > > -        rb_hash_aset(hash, rb_str_new2("hostname"),
> > > -                     rb_str_new2(lease->hostname));
> > > -        rb_hash_aset(hash, rb_str_new2("clientid"),
> > > -                     rb_str_new2(lease->clientid));
> > > +        if (lease->hostname) {
> > > +            rb_hash_aset(hash, rb_str_new2("hostname"),
> > > +                         rb_str_new2(lease->hostname));
> > > +        }
> > > +        if (lease->clientid) {
> > > +            rb_hash_aset(hash, rb_str_new2("clientid"),
> > > +                         rb_str_new2(lease->clientid));
> > > +        }
> > >   
> > >           rb_ary_store(result, i, hash);
> > >       }
> > --
> > libvir-list mailing list
> > libvir-list at redhat.com
> > https://www.redhat.com/mailman/listinfo/libvir-list
> > 
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list