On Thu, 2011-08-11 at 13:40 +0800, walimis wrote:
> On Thu, Aug 11, 2011 at 08:37:01AM +0300, Sasha Levin wrote:
> >On Thu, 2011-08-11 at 13:07 +0800, Liming Wang wrote:
> >> If num_pages is negative, balloon will make kernel crash with
> >> "out of memory". So we check this value to avoid it to be negative.
> >> 
> >> Signed-off-by: Liming Wang <[email protected]>
> >> ---
> >>  tools/kvm/virtio/balloon.c |    7 ++++++-
> >>  1 files changed, 6 insertions(+), 1 deletions(-)
> >> 
> >> diff --git a/tools/kvm/virtio/balloon.c b/tools/kvm/virtio/balloon.c
> >> index 854d04b..0223ee4 100644
> >> --- a/tools/kvm/virtio/balloon.c
> >> +++ b/tools/kvm/virtio/balloon.c
> >> @@ -222,8 +222,13 @@ static void handle_sigmem(int sig)
> >>  {
> >>    if (sig == SIGKVMADDMEM)
> >>            bdev.config.num_pages += 256;
> >> -  else
> >> +  else {
> >>            bdev.config.num_pages -= 256;
> >> +          if ((s32)bdev.config.num_pages < 0){
> >
> >imo it's worth doing this check before the decrement instead of casting
> >to signed here.
> You mean that check whether num_pages less than 256 or equal to 0?
> 

If it's less than 256.

> >
> >you also need to wrap the 'if ()' with parenthesis if you add them to
> >the 'else' case.
> Sorry, I'm not clear what that does mean?
> 

It's a coding style thing. If you have braces in one branch you should
have them in all branches.

For example:

if (something)
        do_this();
else {
        do_this();
        do_that();
}

should be:

if (something) {
        do_this();
} else {
        do_this();
        do_that();
}

We follow the kernel coding style, you can find the full documentation
under the kernel tree in Documentation/CodingStyle .
> walimis
> >
> >> +                  bdev.config.num_pages = 0;
> >> +                  return;
> >> +          }
> >> +  }
> >>  
> >>    /* Notify that the configuration space has changed */
> >>    bdev.isr = VIRTIO_PCI_ISR_CONFIG;
> >
> >-- 
> >
> >Sasha.
> >

-- 

Sasha.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to