Re: [LTP] [PATCH] clone06: fix error of TERM environment variable is not available

Re: [LTP] [PATCH] clone06: fix error of TERM environment variable is not available

[Salvatore CRO']

On 2/14/2012 4:33 AM, Peng Haitao wrote:
> 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.

> Signed-off-by: Peng Haitao<penght@cn.fujitsu.com>
> ---
>   testcases/kernel/syscalls/clone/clone06.c |    9 +++++++++
>   1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/clone/clone06.c b/testcases/kernel/syscalls/clone/clone06.c
> index 1734776..5701e4a 100644
> --- a/testcases/kernel/syscalls/clone/clone06.c
> +++ b/testcases/kernel/syscalls/clone/clone06.c
> @@ -165,6 +165,15 @@ int main(int ac, char **av)
>   void setup()
>   {
>
> +	char *term;
> +
> +	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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] Fwd: Re: [PATCH] clone06: fix error of TERM environment variable is not available

[Salvatore CRO']

On 2/14/2012 9:48 AM, Peng Haitao wrote:
> Salvatore CRO' said the following on 2012-2-14 16:27:
>> On 2/14/2012 9:18 AM, Peng Haitao wrote:
>>> Salvatore CRO' said the following on 2012-2-14 16:04:
>>>> 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!
>>>>
>>> :)
>>>
>>>> I didn't mean in any way to be offensive.
>>>>
>>> Yeah，no problem.
>> Thank you.
>>
>>>> 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.
>>>
>>> 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.
>>
> 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.



>
> Best Regards,
> Peng
>


------------------------------------------------------------------------------
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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list