Skip to contentSkip to conversation

kvm: close opened file

PATCH Superseded Cong Ding 3 participants 9 messages Jan 16, 2013 – Jan 22, 2013 KVM

lore
Share & tools
lore

Cong Ding

author
#0 lore raw

PATCH 1/1 kvm: close opened file

The file should be closed before return.

---
 tools/kvm/builtin-setup.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
tools/kvm/builtin-setup.c +3 -3
index c5b0566..8b45c56 100644
@@ -159,12 +159,12 @@ static int copy_passwd(const char *guestfs_name)
 		return -1;
 
 	ret = fprintf(file, "root:x:0:0:root:/root:/bin/sh\n");
-	if (ret < 0)
-		return ret;
+	if (ret > 0)
+		ret = 0;
 
 	fclose(file);
 
-	return 0;
+	return ret;
 }
 
 static int make_guestfs_symlink(const char *guestfs_name, const char *path)
Signature · 1 line
1.7.10.4
author
#1 lore raw

PATCH 1/1 kvm: remove redundant "if" condition

After we check (state.kcount != 0), state.kcount has to be 0 in all the "else" branchs.

---
 tools/kvm/hw/i8042.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/kvm/hw/i8042.c +1 -1
index 9f8be6a..9035732 100644
@@ -189,7 +189,7 @@ static u32 kbd_read_data(void)
 		state.mcount--;
 		kvm__irq_line(state.kvm, AUX_IRQ, 0);
 		kbd_update_irq();
-	} else if (state.kcount == 0) {
+	} else {
 		i = state.kread - 1;
 		if (i < 0)
 			i = QUEUE_SIZE;
Signature · 1 line
1.7.10.4
#2 lore raw
In reply to Cong Ding

Please use "kvmtool:" prefix at the subject line.

Quoted 34 lines · Cong Ding

The file should be closed before return.

---
 tools/kvm/builtin-setup.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
tools/kvm/builtin-setup.c +3 -3
index c5b0566..8b45c56 100644
@@ -159,12 +159,12 @@ static int copy_passwd(const char *guestfs_name)
 		return -1;
 
 	ret = fprintf(file, "root:x:0:0:root:/root:/bin/sh\n");
-	if (ret < 0)
-		return ret;
+	if (ret > 0)
+		ret = 0;
 
 	fclose(file);
 
-	return 0;
+	return ret;
 }
 
 static int make_guestfs_symlink(const char *guestfs_name, const char *path)
Signature · 6 lines
1.7.10.4

--
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

--
Gleb.

author
#3 lore raw

PATCH v2 1/1 kvm tools: close opened file

The file should be closed before return.

---
 tools/kvm/builtin-setup.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
tools/kvm/builtin-setup.c +3 -3
index c5b0566..8b45c56 100644
@@ -159,12 +159,12 @@ static int copy_passwd(const char *guestfs_name)
 		return -1;
 
 	ret = fprintf(file, "root:x:0:0:root:/root:/bin/sh\n");
-	if (ret < 0)
-		return ret;
+	if (ret > 0)
+		ret = 0;
 
 	fclose(file);
 
-	return 0;
+	return ret;
 }
 
 static int make_guestfs_symlink(const char *guestfs_name, const char *path)
Signature · 1 line
1.7.10.4
author
#5 lore raw

PATCH v2 1/1 kvm tools: remove redundant "if" condition

In reply to Gleb Natapov

After we check (state.kcount != 0), state.kcount has to be 0 in all the "else" branchs.

---
 tools/kvm/hw/i8042.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/kvm/hw/i8042.c +1 -1
index 9f8be6a..9035732 100644
@@ -189,7 +189,7 @@ static u32 kbd_read_data(void)
 		state.mcount--;
 		kvm__irq_line(state.kvm, AUX_IRQ, 0);
 		kbd_update_irq();
-	} else if (state.kcount == 0) {
+	} else {
 		i = state.kread - 1;
 		if (i < 0)
 			i = QUEUE_SIZE;
Signature · 1 line
1.7.10.4
#6 lore raw

Re: [PATCH v2] kvm tools: remove redundant "if" condition

Quoted 13 lines · Cong Ding

After we check (state.kcount != 0), state.kcount has to be 0 in all the "else" branchs.

---
 tools/kvm/hw/i8042.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/kvm/hw/i8042.c +0 -0
index 9f8be6a..9035732 100644

tools/kvm/hw/i8042.c static u32 kbd_read_data(void) · line 195

Quoted 8 lines
tools/kvm/hw/i8042.c +1 -1
@@ -189,7 +189,7 @@ static u32 kbd_read_data(void)
                state.mcount--;
                kvm__irq_line(state.kvm, AUX_IRQ, 0);
                kbd_update_irq();
-       } else if (state.kcount == 0) {
+       } else {
                i = state.kread - 1;
                if (i < 0)
                        i = QUEUE_SIZE;

This doesn't look right. The 'kcount' field is an int so the value can be negative.

author
#7 lore raw

Re: [PATCH v2] kvm tools: remove redundant "if" condition

Quoted 9 lines · Pekka Enberg
Quoted 8 lines · Cong Ding

After we check (state.kcount != 0), state.kcount has to be 0 in all the "else" branchs.

---
 tools/kvm/hw/i8042.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 16 lines

tools/kvm/hw/i8042.c static u32 kbd_read_data(void) · line 195

Quoted 13 lines
tools/kvm/hw/i8042.c +1 -1
index 9f8be6a..9035732 100644
@@ -189,7 +189,7 @@ static u32 kbd_read_data(void)
                state.mcount--;
                kvm__irq_line(state.kvm, AUX_IRQ, 0);
                kbd_update_irq();
-       } else if (state.kcount == 0) {
+       } else {
                i = state.kread - 1;
                if (i < 0)
                        i = QUEUE_SIZE;

This doesn't look right. The 'kcount' field is an int so the value can be negative.

But the former check is "state.kcount != 0" as I described in the commit message. Notice the difference between variable names in the "if" condition: the first one is kcount, the second one is mcount, and the third one is same as the first one kcount.

Ok, the original code is

	if (state.kcount != 0) {
		/* do something when (state.kcount != 0) */
	} else if (state.mcount > 0) {
		/* do something when (state.kcount == 0 && state.mount > 0) */
	} else if (state.kcount == 0) {
		/* do something when (state.kcount == 0 && state.mount <= 0) */
	}

For the third branch, it runs when state.kcount == 0 and state.mount <= 0, it's not necessary to ensure state.kcount == 0 again.

- cong

#8 lore raw

Re: [PATCH v2] kvm tools: remove redundant "if" condition

Quoted 39 lines · Cong Ding
Quoted 9 lines · Pekka Enberg
Quoted 8 lines · Cong Ding

After we check (state.kcount != 0), state.kcount has to be 0 in all the "else" branchs.

---
 tools/kvm/hw/i8042.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 16 lines

tools/kvm/hw/i8042.c static u32 kbd_read_data(void) · line 195

Quoted 13 lines
tools/kvm/hw/i8042.c +1 -1
index 9f8be6a..9035732 100644
@@ -189,7 +189,7 @@ static u32 kbd_read_data(void)
                state.mcount--;
                kvm__irq_line(state.kvm, AUX_IRQ, 0);
                kbd_update_irq();
-       } else if (state.kcount == 0) {
+       } else {
                i = state.kread - 1;
                if (i < 0)
                        i = QUEUE_SIZE;

This doesn't look right. The 'kcount' field is an int so the value can be negative.

But the former check is "state.kcount != 0" as I described in the commit message. Notice the difference between variable names in the "if" condition: the first one is kcount, the second one is mcount, and the third one is same as the first one kcount.

Ok, the original code is

        if (state.kcount != 0) {
                /* do something when (state.kcount != 0) */
        } else if (state.mcount > 0) {
                /* do something when (state.kcount == 0 && state.mount > 0) */
        } else if (state.kcount == 0) {
                /* do something when (state.kcount == 0 && state.mount <= 0) */
        }
Quoted 2 lines

For the third branch, it runs when state.kcount == 0 and state.mount <= 0, it's not necessary to ensure state.kcount == 0 again.

Right you are. Applied, thanks!