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 <walimisdev@xxxxxxxxx>
> >> ---
> >> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
