| Message ID | 5d19aa93-b421-eb1f-3a4c-92fc9476747a@infradead.org (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
On Mon, Jan 22, 2018 at 11:05 AM, Randy Dunlap <rdunlap@infradead.org> wrote: > From: Randy Dunlap <rdunlap@infradead.org> > > In error(), quiet is undefined when the command: > $ ./test-suite format filename1.c foobar1 > > is used and filename1.c does not exist. This causes a shell script error: > ./test-suite: line 147: [: : integer expression expected > > because $quiet is undefined in > [ "$quiet" -ne 1 ] && echo "error: $1" > so the error message is lost. > > Just initialize quiet before any command line parsing. > > Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > --- > validation/test-suite | 2 ++ > 1 file changed, 2 insertions(+) > > --- sprs.orig/validation/test-suite > +++ sprs.next/validation/test-suite > @@ -368,6 +368,8 @@ arg_file() > return 0 > } > > +quiet=0 > + Good catch. I think this fix is not ideal. There is another quite=0 inside "do_test()". <quote> quiet=0 [ $must_fail -eq 1 ] && [ $V -eq 0 ] && quiet=1 </quote> The "quite=0" only cover the case inside "do_test()". Even with this patch, If the "quite" is used outside of "do_test()", The "quite" variable is not set properly according to "V". I would suggest move the above two quote line out side of "do_test()", right after the default value of "V", in the beginning of the file: <quote> # defaults to not verbose [ -z "$V" ] && V=0 </quote> Chris -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/23/2018 03:22 AM, Christopher Li wrote: > On Mon, Jan 22, 2018 at 11:05 AM, Randy Dunlap <rdunlap@infradead.org> wrote: >> From: Randy Dunlap <rdunlap@infradead.org> >> >> In error(), quiet is undefined when the command: >> $ ./test-suite format filename1.c foobar1 >> >> is used and filename1.c does not exist. This causes a shell script error: >> ./test-suite: line 147: [: : integer expression expected >> >> because $quiet is undefined in >> [ "$quiet" -ne 1 ] && echo "error: $1" >> so the error message is lost. >> >> Just initialize quiet before any command line parsing. >> >> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> >> --- >> validation/test-suite | 2 ++ >> 1 file changed, 2 insertions(+) >> >> --- sprs.orig/validation/test-suite >> +++ sprs.next/validation/test-suite >> @@ -368,6 +368,8 @@ arg_file() >> return 0 >> } >> >> +quiet=0 >> + > > Good catch. I think this fix is not ideal. > There is another quite=0 inside "do_test()". > <quote> > quiet=0 > [ $must_fail -eq 1 ] && [ $V -eq 0 ] && quiet=1 > </quote> > > The "quite=0" only cover the case inside "do_test()". > Even with this patch, If the "quite" is used outside of > "do_test()", The "quite" variable is not set properly according to "V". > > I would suggest move the above two quote line out side > of "do_test()", right after the default value of "V", in the > beginning of the file: > > <quote> > # defaults to not verbose > [ -z "$V" ] && V=0 > </quote> At the beginning of the file, $must_fail is not defined, so it will not -eq 1, so quiet will remain = 0. I guess that I don't understand the interplay between $V and $quiet and why have both of them. Are you suggesting that $quiet should be initialized by default like this: (just drop the $must_fail part) > quiet=0 > [ $V -eq 0 ] && quiet=1 thanks,
On Tue, Jan 23, 2018 at 10:20:55AM -0800, Randy Dunlap wrote: > > At the beginning of the file, $must_fail is not defined, so it will not -eq 1, > so quiet will remain = 0. > > I guess that I don't understand the interplay between $V and $quiet and why > have both of them. $V is something global, set once for all when starting the test suite. $quiet is something that can change for each file being tested. > Are you suggesting that $quiet should be initialized by default like this: > (just drop the $must_fail part) > > > quiet=0 > > [ $V -eq 0 ] && quiet=1 The whole can probably be simplified a bit but the idea with $must_fail (set for test cases we known/expected to fail) is that we're not interested in any sort of details about the failure of a test that is supposed to fail. Regards, -- Luc -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 22, 2018 at 11:05:09AM -0800, Randy Dunlap wrote: > From: Randy Dunlap <rdunlap@infradead.org> > > In error(), quiet is undefined when the command: > $ ./test-suite format filename1.c foobar1 > > is used and filename1.c does not exist. This causes a shell script error: > ./test-suite: line 147: [: : integer expression expected > > because $quiet is undefined in > [ "$quiet" -ne 1 ] && echo "error: $1" > so the error message is lost. > > Just initialize quiet before any command line parsing. > > Signed-off-by: Randy Dunlap <rdunlap@infradead.org> cfr: https://marc.info/?l=linux-sparse&m=151273892009415 -- Luc -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/24/2018 12:28 AM, Luc Van Oostenryck wrote: > On Mon, Jan 22, 2018 at 11:05:09AM -0800, Randy Dunlap wrote: >> From: Randy Dunlap <rdunlap@infradead.org> >> >> In error(), quiet is undefined when the command: >> $ ./test-suite format filename1.c foobar1 >> >> is used and filename1.c does not exist. This causes a shell script error: >> ./test-suite: line 147: [: : integer expression expected >> >> because $quiet is undefined in >> [ "$quiet" -ne 1 ] && echo "error: $1" >> so the error message is lost. >> >> Just initialize quiet before any command line parsing. >> >> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > > cfr: https://marc.info/?l=linux-sparse&m=151273892009415 I don't think that will fix the original problem that I posted the patch for.
On Wed, Jan 24, 2018 at 9:48 AM, Randy Dunlap <rdunlap@infradead.org> wrote: > I don't think that will fix the original problem that I posted the > patch for. Agree, as long as the $quiet is set inside do_test(), any call to error() out side of do_test() will cause error. That is the original problem. Chris -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 24, 2018 at 09:48:07AM -0800, Randy Dunlap wrote: > On 01/24/2018 12:28 AM, Luc Van Oostenryck wrote: > > On Mon, Jan 22, 2018 at 11:05:09AM -0800, Randy Dunlap wrote: > >> From: Randy Dunlap <rdunlap@infradead.org> > >> > >> In error(), quiet is undefined when the command: > >> $ ./test-suite format filename1.c foobar1 > >> > >> is used and filename1.c does not exist. This causes a shell script error: > >> ./test-suite: line 147: [: : integer expression expected > >> > >> because $quiet is undefined in > >> [ "$quiet" -ne 1 ] && echo "error: $1" > >> so the error message is lost. > >> > >> Just initialize quiet before any command line parsing. > >> > >> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > > > > cfr: https://marc.info/?l=linux-sparse&m=151273892009415 > > I don't think that will fix the original problem that I posted the > patch for. Yes, you're right. The original problem doesn't exist in my tree but for other reasons. -- Luc -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 24, 2018 at 08:19:23PM +0100, Luc Van Oostenryck wrote: > On Wed, Jan 24, 2018 at 09:48:07AM -0800, Randy Dunlap wrote: > > On 01/24/2018 12:28 AM, Luc Van Oostenryck wrote: > > > On Mon, Jan 22, 2018 at 11:05:09AM -0800, Randy Dunlap wrote: > > >> From: Randy Dunlap <rdunlap@infradead.org> > > >> > > >> In error(), quiet is undefined when the command: > > >> $ ./test-suite format filename1.c foobar1 > > >> > > >> is used and filename1.c does not exist. This causes a shell script error: > > >> ./test-suite: line 147: [: : integer expression expected > > >> > > >> because $quiet is undefined in > > >> [ "$quiet" -ne 1 ] && echo "error: $1" > > >> so the error message is lost. > > >> > > >> Just initialize quiet before any command line parsing. > > >> > > >> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > > > > > > cfr: https://marc.info/?l=linux-sparse&m=151273892009415 > > > > I don't think that will fix the original problem that I posted the > > patch for. > > Yes, you're right. > The original problem doesn't exist in my tree but for another reason. Nevertheless, the reason explained in my patch is still valid and $quiet also need to be reset early in do_test(). -- Luc -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- sprs.orig/validation/test-suite +++ sprs.next/validation/test-suite @@ -368,6 +368,8 @@ arg_file() return 0 } +quiet=0 + case "$1" in '') do_test_suite