Skip to contentSkip to conversation

clone06: legitimate zero as sprintf return value

PATCH In review Salvatore CRO' 3 participants 8 messages Feb 14, 2012 – Feb 29, 2012 1 message not in the archive

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 clone06: legitimate zero as sprintf return value

A return value of 0 for sprintf is acceptable as it means zero byte read.
Indeed errno is not set in this case thus no warning messages must be issued.

---
 testcases/kernel/syscalls/clone/clone06.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
testcases/kernel/syscalls/clone/clone06.c +1 -1
index 5db183b..8ba7c13 100644
@@ -199,7 +199,7 @@ int child_environ(void)
 		tst_brkm(TBROK|TERRNO, cleanup, "close(pfd[0]) failed");
 	}
 
-	if ((sprintf(var, "%s", getenv("TERM") ? : "")) <= 0) {
+	if ((sprintf(var, "%s", getenv("TERM") ? : "")) < 0) {
 		tst_resm(TWARN|TERRNO, "sprintf() failed");
 	}
 
Signature · 13 lines
1.7.4.4


------------------------------------------------------------------------------
Keep Your Developer Skills Current with LearnDevNow!
The most comprehensive online learning library for Microsoft developers
is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3,
Metro Style Apps, more. Free future releases when you subscribe now!
http://p.sf.net/sfu/learndevnow-d2d
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
#1 lore raw

PATCH 1/1 clone06: legitimate zero as sprintf return value

To be honest I don't see any reason to have the sprintf at all. I will refine my patch to directly evaluate string var to getenv...

-------- Original Message --------
Return-Path: <[email protected]>
Received: from localhost (lxmcdt5.ctn.st.com [164.130.129.175]) by
mail7.sgp.st.com (MOS 4.3.3-GA) with ESMTP id ALM11201 (AUTH cros); Tue, 14 Feb 2012 11:33:12 +0100
From: Salvatore CRO' <[email protected]>
To: [email protected]
Cc: Salvatore Cro <[email protected]>
Subject: [PATCH] clone06: legitimate zero as sprintf return value
Date: Tue, 14 Feb 2012 11:33:01 +0100
Message-Id: <[email protected]>
X-Mailer: git-send-email 1.7.4.4

A return value of 0 for sprintf is acceptable as it means zero byte read.
Indeed errno is not set in this case thus no warning messages must be issued.

---
  testcases/kernel/syscalls/clone/clone06.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
testcases/kernel/syscalls/clone/clone06.c +1 -1
index 5db183b..8ba7c13 100644
@@ -199,7 +199,7 @@ int child_environ(void)
  		tst_brkm(TBROK|TERRNO, cleanup, "close(pfd[0]) failed");
  	}
 
-	if ((sprintf(var, "%s", getenv("TERM") ? : ""))<= 0) {
+	if ((sprintf(var, "%s", getenv("TERM") ? : ""))<  0) {
  		tst_resm(TWARN|TERRNO, "sprintf() failed");
  	}
 
Signature · 14 lines
1.7.4.4



------------------------------------------------------------------------------
Keep Your Developer Skills Current with LearnDevNow!
The most comprehensive online learning library for Microsoft developers
is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3,
Metro Style Apps, more. Free future releases when you subscribe now!
http://p.sf.net/sfu/learndevnow-d2d
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
#2 lore raw

PATCH v2 1/1 clone06: set child_env to getenv directly

In reply to Salvatore CRO'

sprintf check is bugged as a return value of 0 is acceptable. Furthermore sprintf to set the child env var is unneeded. Drop sprintf call as child_env can be set to getenv directly.

---
 testcases/kernel/syscalls/clone/clone06.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)
testcases/kernel/syscalls/clone/clone06.c +3 -5
index 5db183b..50c8ef3 100644
@@ -192,18 +192,16 @@ void cleanup()
 int child_environ(void)
 {
 
-	char var[MAX_LINE_LENGTH];
+	char *child_env;
 
 	/* Close read end from child */
 	if ((close(pfd[0])) == -1) {
 		tst_brkm(TBROK|TERRNO, cleanup, "close(pfd[0]) failed");
 	}
 
-	if ((sprintf(var, "%s", getenv("TERM") ? : "")) <= 0) {
-		tst_resm(TWARN|TERRNO, "sprintf() failed");
-	}
+	child_env = getenv("TERM") ? : "";
 
-	if ((write(pfd[1], var, MAX_LINE_LENGTH)) == -1) {
+	if ((write(pfd[1], child_env, MAX_LINE_LENGTH)) == -1) {
 		tst_resm(TWARN|TERRNO, "write to pipe failed");
 	}
 
Signature · 13 lines
1.7.4.4


------------------------------------------------------------------------------
Keep Your Developer Skills Current with LearnDevNow!
The most comprehensive online learning library for Microsoft developers
is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3,
Metro Style Apps, more. Free future releases when you subscribe now!
http://p.sf.net/sfu/learndevnow-d2d
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
Acked-by on v2 1/1
#3 lore raw

Re: [LTP] [PATCH V2] clone06: set child_env to getenv directly

Quoted 5 lines · Salvatore CRO

sprintf check is bugged as a return value of 0 is acceptable. Furthermore sprintf to set the child env var is unneeded. Drop sprintf call as child_env can be set to getenv directly.

Quoted 20 lines
---
 testcases/kernel/syscalls/clone/clone06.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)
testcases/kernel/syscalls/clone/clone06.c +1 -1
index 5db183b..50c8ef3 100644
@@ -192,18 +192,16 @@ void cleanup()
 int child_environ(void)
 {
 
-	char var[MAX_LINE_LENGTH];
+	char *child_env;
 
 	/* Close read end from child */
 	if ((close(pfd[0])) == -1) {
 		tst_brkm(TBROK|TERRNO, cleanup, "close(pfd[0]) failed");
 	}
 

testcases/kernel/syscalls/clone/clone06.c void cleanup() · line 207

Quoted 10 lines
testcases/kernel/syscalls/clone/clone06.c +2 -4
-	if ((sprintf(var, "%s", getenv("TERM") ? : "")) <= 0) {
-		tst_resm(TWARN|TERRNO, "sprintf() failed");
-	}
+	child_env = getenv("TERM") ? : "";
 
-	if ((write(pfd[1], var, MAX_LINE_LENGTH)) == -1) {
+	if ((write(pfd[1], child_env, MAX_LINE_LENGTH)) == -1) {
 		tst_resm(TWARN|TERRNO, "write to pipe failed");
 	}
 

------------------------------------------------------------------------------
Keep Your Developer Skills Current with LearnDevNow!
The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now!
http://p.sf.net/sfu/learndevnow-d2d
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

#4 lore raw

Re: [LTP] [PATCH V2] clone06: set child_env to getenv directly

In reply to Salvatore CRO'

Hi!

Quoted 19 lines
---
 testcases/kernel/syscalls/clone/clone06.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)
testcases/kernel/syscalls/clone/clone06.c +1 -1
index 5db183b..50c8ef3 100644
@@ -192,18 +192,16 @@ void cleanup()
 int child_environ(void)
 {
 
-	char var[MAX_LINE_LENGTH];
+	char *child_env;
 
 	/* Close read end from child */
 	if ((close(pfd[0])) == -1) {
 		tst_brkm(TBROK|TERRNO, cleanup, "close(pfd[0]) failed");

testcases/kernel/syscalls/clone/clone06.c void cleanup() · line 205

Quoted 10 lines
testcases/kernel/syscalls/clone/clone06.c +2 -4
 	}
 
-	if ((sprintf(var, "%s", getenv("TERM") ? : "")) <= 0) {
-		tst_resm(TWARN|TERRNO, "sprintf() failed");
-	}
+	child_env = getenv("TERM") ? : "";
 
-	if ((write(pfd[1], var, MAX_LINE_LENGTH)) == -1) {
+	if ((write(pfd[1], child_env, MAX_LINE_LENGTH)) == -1) {
 		tst_resm(TWARN|TERRNO, "write to pipe failed");

Hmm, that one is wrong too, as you write MAX_LINE_LENGTH bytes you write bytest __after__ the buffer.

Signature · 12 lines
Cyril Hrubis
[email protected]

------------------------------------------------------------------------------
Virtualization & Cloud Management Using Capacity Planning
Cloud computing makes use of virtualization - but cloud computing 
also focuses on allowing computing to be delivered as a service.
http://www.accelacomm.com/jaw/sfnl/114/51521223/
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
#5 lore raw

Re: [LTP] [PATCH V2] clone06: set child_env to getenv directly

Hi!

Quoted 2 lines

Hmm, that one is wrong too, as you write MAX_LINE_LENGTH bytes you write bytest __after__ the buffer.

bytes from __after__ the buffer.

Signature · 12 lines
Cyril Hrubis
[email protected]

------------------------------------------------------------------------------
Virtualization & Cloud Management Using Capacity Planning
Cloud computing makes use of virtualization - but cloud computing 
also focuses on allowing computing to be delivered as a service.
http://www.accelacomm.com/jaw/sfnl/114/51521223/
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

A message that is not in the archive (sent off-list or not yet received): [email protected]

#7 lore raw

Re: [LTP] [PATCH V2] clone06: set child_env to getenv directly

Hi!

Quoted 35 lines
Quoted 21 lines
Quoted 21 lines
testcases/kernel/syscalls/clone/clone06.c +2 -4
index 5db183b..50c8ef3 100644
@@ -192,18 +192,16 @@ void cleanup()
 int child_environ(void)
 {
 
-	char var[MAX_LINE_LENGTH];
+	char *child_env;
 
 	/* Close read end from child */
 	if ((close(pfd[0])) == -1) {
 		tst_brkm(TBROK|TERRNO, cleanup, "close(pfd[0]) failed");
 	}
 
-	if ((sprintf(var, "%s", getenv("TERM") ? : ""))<= 0) {
-		tst_resm(TWARN|TERRNO, "sprintf() failed");
-	}
+	child_env = getenv("TERM") ? : "";
 
Quoted 6 lines

testcases/kernel/syscalls/clone/clone06.c void cleanup() · line 205

Quoted 3 lines
testcases/kernel/syscalls/clone/clone06.c +1 -1
-	if ((write(pfd[1], var, MAX_LINE_LENGTH)) == -1) {
+	if ((write(pfd[1], child_env, MAX_LINE_LENGTH)) == -1) {
 		tst_resm(TWARN|TERRNO, "write to pipe failed");

Hmm, that one is wrong too, as you write MAX_LINE_LENGTH bytes you write bytest __after__ the buffer.

I see...
Whould then original patch

testcases/kernel/syscalls/clone/clone06.c +1 -1
-       if ((sprintf(var, "%s", getenv("TERM") ? : "")) <= 0) {
+       if ((sprintf(var, "%s", getenv("TERM") ? : "")) < 0) {
                tst_resm(TWARN|TERRNO, "sprintf() failed");
        }
Quoted 2 lines

sounds more suitable?
If yes you can directly apply it. Let me know if it needs me to resend it.

That one seems to be fine.

Signature · 12 lines
Cyril Hrubis
[email protected]

------------------------------------------------------------------------------
Virtualization & Cloud Management Using Capacity Planning
Cloud computing makes use of virtualization - but cloud computing 
also focuses on allowing computing to be delivered as a service.
http://www.accelacomm.com/jaw/sfnl/114/51521223/
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
#8 lore raw
In reply to Salvatore CRO'

Hi!

testcases/kernel/syscalls/clone/clone06.c int child_environ(void) · line 203

Quoted 3 lines
testcases/kernel/syscalls/clone/clone06.c +1 -1
-	if ((sprintf(var, "%s", getenv("TERM") ? : "")) <= 0) {
+	if ((sprintf(var, "%s", getenv("TERM") ? : "")) < 0) {
 		tst_resm(TWARN|TERRNO, "sprintf() failed");

Commited, thanks.

Signature · 12 lines
Cyril Hrubis
[email protected]

------------------------------------------------------------------------------
Virtualization & Cloud Management Using Capacity Planning
Cloud computing makes use of virtualization - but cloud computing 
also focuses on allowing computing to be delivered as a service.
http://www.accelacomm.com/jaw/sfnl/114/51521223/
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list