All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH] netns: remove accumulated errmesg assignment
@ 2013-11-25  6:47 Monson Shao
  2013-11-26 18:28 ` chrubis
  0 siblings, 1 reply; 3+ messages in thread
From: Monson Shao @ 2013-11-25  6:47 UTC (permalink / raw)
  To: ltp-list

Valuable $errmesg wasn't initialized at beginning, when bash option
"nounset" is set, statement like errmesg="$errmesg XXXXXX" will cause
bash unbound variable error.

Furthermore, $errmesg will be echoed every time it is assigned, so
there is no need for accumulated assignment.

Signed-off-by: Monson Shao <jshao@redhat.com>
---
 testcases/kernel/containers/netns/runnetnstest.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/testcases/kernel/containers/netns/runnetnstest.sh b/testcases/kernel/containers/netns/runnetnstest.sh
index fb494a2..f7bf80f 100755
--- a/testcases/kernel/containers/netns/runnetnstest.sh
+++ b/testcases/kernel/containers/netns/runnetnstest.sh
@@ -60,7 +60,7 @@ two_children_ns
 rc=$?
 if [ $rc -ne 0 ]; then
     exit_code=$rc
-    errmesg="$errmesg two_children_ns: return code is $exit_code ; "
+    errmesg="two_children_ns: return code is $exit_code ; "
     echo $errmesg
 else
    echo "two_children_ns: PASS"
@@ -71,7 +71,7 @@ crtchild_delchild
 rc=$?
 if [ $rc -ne 0 ]; then
     exit_code=$rc
-    errmesg="$errmesg crtchild_delchild: return code is $exit_code ; "
+    errmesg="crtchild_delchild: return code is $exit_code ; "
     echo $errmesg
 else
    echo "crtchild_delchild: PASS"
@@ -83,7 +83,7 @@ par_chld_ipv6
 rc=$?
 if [ $rc -ne 0 ]; then
     exit_code=$rc
-    errmesg="$errmesg par_chld_ipv6: return code is $exit_code ; "
+    errmesg="par_chld_ipv6: return code is $exit_code ; "
     echo $errmesg
 else
    echo "par_chld_ipv6: PASS"
@@ -99,7 +99,7 @@ echo
 #rc=$?
 #if [ $rc -ne 0 ]; then
 #    exit_code=$rc
-#    errmesg="$errmesg sysfsview: return code is $exit_code ; "
+#    errmesg="sysfsview: return code is $exit_code ; "
 #    echo $errmesg
 #else
 #   echo "sysfsview: PASS"
@@ -119,7 +119,7 @@ par_chld_ftp
 rc=$?
 if [ $rc -ne 0 ]; then
     exit_code=$rc
-    errmesg="$errmesg par_chld_ftp: FAIL $exit_code ; "
+    errmesg="par_chld_ftp: FAIL $exit_code ; "
     echo $errmesg
 else
    echo "par_chld_ftp: PASS"
-- 
1.8.3.1


------------------------------------------------------------------------------
Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing 
conversations that shape the rapidly evolving mobile landscape. Sign up now. 
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [LTP] [PATCH] netns: remove accumulated errmesg assignment
  2013-11-25  6:47 [LTP] [PATCH] netns: remove accumulated errmesg assignment Monson Shao
@ 2013-11-26 18:28 ` chrubis
       [not found]   ` <20131127094113.GR14611@redhat.com>
  0 siblings, 1 reply; 3+ messages in thread
From: chrubis @ 2013-11-26 18:28 UTC (permalink / raw)
  To: Monson Shao; +Cc: ltp-list

Hi!
> Valuable $errmesg wasn't initialized at beginning, when bash option
> "nounset" is set, statement like errmesg="$errmesg XXXXXX" will cause
> bash unbound variable error.
> 
> Furthermore, $errmesg will be echoed every time it is assigned, so
> there is no need for accumulated assignment.
> 
> Signed-off-by: Monson Shao <jshao@redhat.com>
> ---
>  testcases/kernel/containers/netns/runnetnstest.sh | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/testcases/kernel/containers/netns/runnetnstest.sh b/testcases/kernel/containers/netns/runnetnstest.sh
> index fb494a2..f7bf80f 100755
> --- a/testcases/kernel/containers/netns/runnetnstest.sh
> +++ b/testcases/kernel/containers/netns/runnetnstest.sh
> @@ -60,7 +60,7 @@ two_children_ns
>  rc=$?
>  if [ $rc -ne 0 ]; then
>      exit_code=$rc
> -    errmesg="$errmesg two_children_ns: return code is $exit_code ; "
> +    errmesg="two_children_ns: return code is $exit_code ; "
>      echo $errmesg
>  else
>     echo "two_children_ns: PASS"
> @@ -71,7 +71,7 @@ crtchild_delchild
>  rc=$?
>  if [ $rc -ne 0 ]; then
>      exit_code=$rc
> -    errmesg="$errmesg crtchild_delchild: return code is $exit_code ; "
> +    errmesg="crtchild_delchild: return code is $exit_code ; "
>      echo $errmesg
>  else
>     echo "crtchild_delchild: PASS"
> @@ -83,7 +83,7 @@ par_chld_ipv6
>  rc=$?
>  if [ $rc -ne 0 ]; then
>      exit_code=$rc
> -    errmesg="$errmesg par_chld_ipv6: return code is $exit_code ; "
> +    errmesg="par_chld_ipv6: return code is $exit_code ; "
>      echo $errmesg
>  else
>     echo "par_chld_ipv6: PASS"
> @@ -99,7 +99,7 @@ echo
>  #rc=$?
>  #if [ $rc -ne 0 ]; then
>  #    exit_code=$rc
> -#    errmesg="$errmesg sysfsview: return code is $exit_code ; "
> +#    errmesg="sysfsview: return code is $exit_code ; "
>  #    echo $errmesg
>  #else
>  #   echo "sysfsview: PASS"
> @@ -119,7 +119,7 @@ par_chld_ftp
>  rc=$?
>  if [ $rc -ne 0 ]; then
>      exit_code=$rc
> -    errmesg="$errmesg par_chld_ftp: FAIL $exit_code ; "
> +    errmesg="par_chld_ftp: FAIL $exit_code ; "
>      echo $errmesg
>  else
>     echo "par_chld_ftp: PASS"

I wonder why there is errmesg variable at all and not just echo, it
doesn't seems to be used anywhere else.

Moreover the way the testcases are executed is not well designed, the
runtest file 'containers' executes script container_test.sh which
executes several scripts and the scripts finally executes the tescases.
This is really nightmare if you are trying to find what fails and why...

It would be much better to execute the testcases directly one by one
from the runtest file, and in most of the cases that would just need to
move code the checks if there is support for particual container to the
testcases.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [LTP] [PATCH] netns: remove accumulated errmesg assignment
       [not found]   ` <20131127094113.GR14611@redhat.com>
@ 2013-11-27 12:50     ` chrubis
  0 siblings, 0 replies; 3+ messages in thread
From: chrubis @ 2013-11-27 12:50 UTC (permalink / raw)
  To: ltp-list

Hi!
> > I wonder why there is errmesg variable at all and not just echo, it
> > doesn't seems to be used anywhere else.
> 
> I wonder too, but I thought there might be some reason, so I only fix
> the error and leave the imperfect behind.

I'm perfectly happy with applying this patch, but I would be even
happier with fixing it up correctly.

> > Moreover the way the testcases are executed is not well designed, the
> > runtest file 'containers' executes script container_test.sh which
> > executes several scripts and the scripts finally executes the tescases.
> > This is really nightmare if you are trying to find what fails and why...
> >
> > It would be much better to execute the testcases directly one by one
> > from the runtest file, and in most of the cases that would just need to
> > move code the checks if there is support for particual container to the
> > testcases.
> 
> Can't agree more, but it takes time to change the whole structure in
> testcases/kernel/containers/netns/ ...

We can do that easily in a few steps. First we need to add checks if the
containers are enabled to the actual testcases (that will not break
anything), then we can add new runtest file for them and once everything
is working, we can finally remove the original scripts.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-11-27 12:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-25  6:47 [LTP] [PATCH] netns: remove accumulated errmesg assignment Monson Shao
2013-11-26 18:28 ` chrubis
     [not found]   ` <20131127094113.GR14611@redhat.com>
2013-11-27 12:50     ` chrubis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.