clone06: fix error of TERM environment variable is not available

PATCH Salvatore CRO' 1 participant 2 messages Feb 14, 2012 5 messages not in the archive

lore

Share & tools

lore

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

Salvatore CRO'

author

2012-02-14 07:55 UTC #1 lore raw

Quoted 6 lines · Peng Haitao

When TERM environment variable is not available, the case may be fail. The patch use the following steps:
1. unset a predetermined env variable if it's present.
2. set it to a known value
3. Make sure that the env variable set in 2. matches both in the child and parent.

Humm... I'm not so keen on this solution as it sounds like a .... IMO.

The case when env variable is not defined should be treated the same way. This
proves once again that both parent and child get the same results.

If your execution ends up with

clone06 0 TWARN : sprintf() failed: errno=???(0): Success

when TERM is not defined, this is because sprintf check is not correct :

         if ((sprintf(var, "%s", getenv("TERM") ? : "")) <= 0) {
                 tst_resm(TWARN|TERRNO, "sprintf() failed");
         }

In this case var is evaluated to "" and sprintf will return "0 byte read" that is quite acceptable.
Child will face an analogue situation so test will pass.

Correct patch should be to change sprintf check to :

if ((sprintf(var, "%s", getenv("TERM") ? : "")) < 0) {
                 tst_resm(TWARN|TERRNO, "sprintf() failed");
         }

I will send it asap...

Ciao,
Salvo.

Quoted 15 lines

Signed-off-by Peng Haitao<[email protected]>

---
  testcases/kernel/syscalls/clone/clone06.c |    9 +++++++++
  1 files changed, 9 insertions(+), 0 deletions(-)

testcases/kernel/syscalls/clone/clone06.c +2 -0

index 1734776..5701e4a 100644
@@ -165,6 +165,15 @@ int main(int ac, char **av)
  void setup()
  {
 
+	char *term;
+

testcases/kernel/syscalls/clone/clone06.c int main(int ac, char **av) · line 179

Quoted 10 lines

testcases/kernel/syscalls/clone/clone06.c +7 -0

+	term = getenv("TERM");
+	if (term)
+		if (unsetenv("TERM") == -1)
+			tst_brkm(TBROK|TERRNO, NULL, "unsetenv failed");
+	if (setenv("TERM", "xterm", 1) == -1)
+		tst_brkm(TBROK|TERRNO, NULL, "setenv failed");
+
  	tst_sig(NOFORK, DEF_HANDLER, cleanup);
 
  	TEST_PAUSE;

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

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

Salvatore CRO'

author

2012-02-14 10:03 UTC #6 lore raw

Quoted 28 lines · Peng Haitao

Salvatore CRO' said the following on 2012-2-14 16:27:
Quoted 19 lines · Peng Haitao
Quoted 11 lines
Salvatore CRO' said the following on 2012-2-14 16:04:
Quoted 5 lines
Sorry for "...." in the first sentence of my reply.
I was looking for the right word to complete the sentence and put "..." in the meanwhile. Then I forgot to do the text replacing!
:)
Quoted 2 lines
I didn't mean in any way to be offensive.
Yeah，no problem.
Thank you.
Quoted 5 lines
Quoted 2 lines
Anyhow I think your solution could not be the proper one.
When TERM env variable is defined, first unsetenv(), then call setenv() to set it. When TERM env variable is not defined, then call setenv("TERM", "xterm", 1) to set it.
Quoted 8 lines
Quoted 2 lines
This has a problem?
I don't see anything wrong with your implementation. On the other hand if the variable is not set we should not force it in my opinion and keep test logic consistent.
If var is ND both child and parent will evaluate respective variable to "" and the test will pass. What we could argue on is whether to still treat the same way the two cases when sprintf returns 0 or negative number. IMO, it should not be the case.

Quoted 5 lines

The case test to verify inheritance of environment variables by child. If environment variable is not set, I think the case should be exit.
I modify the case with the solution, because Garrett Cooper said the steps:)
http://sourceforge.net/mailarchive/message.php?msg_id=27104028

It does not convince me either. The problem Cristian reported refers to how ssh handles user profile initialization.
I had the same problem as I run ltp on an embedded system from linux host via ssh and I did not modifiy the test at all.
The approach Garett proposed to Cristian is not applicable as it modifies the test to overcome a ssh specific issue.
Instead I used ssh -t -t (twice!) when executing ltp to ensure TERM variable is defined in console initialized by ssh.

Furthermore subsequent commit 60eddf837a9c414fe18dded4b07522d0d2066761 made the test safer against
this situation.

Based on this rationale I will propose the patch to modify sprintf check.

BR,
Salvo.

Quoted 4 lines

Best Regards,
Peng