[LTP] [PATCH] quotactl01: Fix tst_resm() format causing crash

[LTP] [PATCH] quotactl01: Fix tst_resm() format causing crash

[Geert Uytterhoeven]

When the quotactl syscall fails, quotactl01 crashes with a segmentation fault
due to an incorrect printf()-style format.

Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
---
 testcases/kernel/syscalls/quotactl/quotactl01.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/testcases/kernel/syscalls/quotactl/quotactl01.c b/testcases/kernel/syscalls/quotactl/quotactl01.c
index 6800a25..0be4fd5 100644
--- a/testcases/kernel/syscalls/quotactl/quotactl01.c
+++ b/testcases/kernel/syscalls/quotactl/quotactl01.c
@@ -147,7 +147,7 @@ int main(int ac, char **av) {
                 for (i = 0; i <= 7; i++){
                         TEST(retval = syscall(__NR_quotactl, cmd[i], (const char *)NULL, id, (caddr_t)NULL));
 			if(TEST_RETURN != 0){
-                 	   	tst_resm(TFAIL, "%s failed - errno = %d : %s ...cmderror=%s", TCID, TEST_ERRNO, strerror(TEST_ERRNO),cmd[i]);
+				tst_resm(TFAIL, "%s failed - errno = %d : %s ...cmd=0x%x", TCID, TEST_ERRNO, strerror(TEST_ERRNO), cmd[i]);
                         }else{
         			tst_resm(TPASS, "quotactl call succeeded");
                         }
-- 
1.6.2.4

With kind regards,

Geert Uytterhoeven
Software Architect
Techsoft Centre

Technology and Software Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH] Annotate tst_*() helpers with __attribute__ ((format (printf, M, N))) (was: Re: [PATCH] quotactl01: Fix tst_resm() format causing crash)

[Geert Uytterhoeven]

On Thu, 27 Aug 2009, Geert Uytterhoeven wrote:
> When the quotactl syscall fails, quotactl01 crashes with a segmentation fault
> due to an incorrect printf()-style format.

This bug encouraged me to add annotations to the test helpers that take
printf()-style formats, cfr. the patch below.

It causes a massive amount of compiler warnings, most of them caused by
TEST_ERRNO being long. According to CVS history, both TEST_RETURN and
TEST_ERRNO have been changed from int to long to accomodate 64-bit platforms,
but to me the change of TEST_ERRNO looks bogus.
As errno is int according to C99, TEST_ERRNO should actually be int too, right? 

Note that there are also a few other cases where integers are used on pointer
type format specifiers. These will cause crashes when the code path is
executed.

From 601578f79c05779acefe023fd499726d9fe4ce03 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Date: Thu, 27 Aug 2009 17:08:18 +0200
Subject: [PATCH] Annotate tst_*() helpers with __attribute__ ((format (printf, M, N)))

Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
---
 include/test.h |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/include/test.h b/include/test.h
index 34ed5d2..03b0d46 100644
--- a/include/test.h
+++ b/include/test.h
@@ -186,12 +186,18 @@
  * Functions from lib/tst_res.c
  */
 const char *strttype(int ttype);
-void tst_res(int ttype, char *fname, char *arg_fmt, ...);
-void tst_resm(int ttype, char *arg_fmt, ...);
-void tst_brk(int ttype, char *fname, void (*func)(void), char *arg_fmt, ...);
-void tst_brkloop(int ttype, char *fname, void (*func)(void), char *arg_fmt, ...);
-void tst_brkm(int ttype, void (*func)(void), char *arg_fmt, ...);
-void tst_brkloopm(int ttype, void (*func)(void), char *arg_fmt, ...);
+void tst_res(int ttype, char *fname, char *arg_fmt, ...)
+	__attribute__ ((format (printf, 3, 4)));
+void tst_resm(int ttype, char *arg_fmt, ...)
+	__attribute__ ((format (printf, 2, 3)));
+void tst_brk(int ttype, char *fname, void (*func)(void), char *arg_fmt, ...)
+	__attribute__ ((format (printf, 4, 5)));
+void tst_brkloop(int ttype, char *fname, void (*func)(void), char *arg_fmt, ...)
+	__attribute__ ((format (printf, 4, 5)));
+void tst_brkm(int ttype, void (*func)(void), char *arg_fmt, ...)
+	__attribute__ ((format (printf, 3, 4)));
+void tst_brkloopm(int ttype, void (*func)(void), char *arg_fmt, ...)
+	__attribute__ ((format (printf, 3, 4)));
 void tst_require_root(void (*func)(void));
 int  tst_environ(void);
 void tst_exit(void) LTP_ATTRIBUTE_NORETURN;
-- 
1.6.2.4

With kind regards,

Geert Uytterhoeven
Software Architect
Techsoft Centre

Technology and Software Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] Annotate tst_*() helpers with __attribute__ ((format (printf, M, N))) (was: Re: [PATCH] quotactl01: Fix tst_resm() format causing crash)

[Subrata Modak]

On Thu, 2009-08-27 at 17:29 +0200, Geert Uytterhoeven wrote: 
> On Thu, 27 Aug 2009, Geert Uytterhoeven wrote:
> > When the quotactl syscall fails, quotactl01 crashes with a segmentation fault
> > due to an incorrect printf()-style format.
> 
> This bug encouraged me to add annotations to the test helpers that take
> printf()-style formats, cfr. the patch below.
> 
> It causes a massive amount of compiler warnings, most of them caused by
> TEST_ERRNO being long. According to CVS history, both TEST_RETURN and
> TEST_ERRNO have been changed from int to long to accomodate 64-bit platforms,
> but to me the change of TEST_ERRNO looks bogus.
> As errno is int according to C99, TEST_ERRNO should actually be int too, right? 
> 
> Note that there are also a few other cases where integers are used on pointer
> type format specifiers. These will cause crashes when the code path is
> executed.
> 
> >From 601578f79c05779acefe023fd499726d9fe4ce03 Mon Sep 17 00:00:00 2001
> From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> Date: Thu, 27 Aug 2009 17:08:18 +0200
> Subject: [PATCH] Annotate tst_*() helpers with __attribute__ ((format (printf, M, N)))
> 
> Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>

Ok. Thanks.

Regards--
Subrata

> ---
>  include/test.h |   18 ++++++++++++------
>  1 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/include/test.h b/include/test.h
> index 34ed5d2..03b0d46 100644
> --- a/include/test.h
> +++ b/include/test.h
> @@ -186,12 +186,18 @@
>   * Functions from lib/tst_res.c
>   */
>  const char *strttype(int ttype);
> -void tst_res(int ttype, char *fname, char *arg_fmt, ...);
> -void tst_resm(int ttype, char *arg_fmt, ...);
> -void tst_brk(int ttype, char *fname, void (*func)(void), char *arg_fmt, ...);
> -void tst_brkloop(int ttype, char *fname, void (*func)(void), char *arg_fmt, ...);
> -void tst_brkm(int ttype, void (*func)(void), char *arg_fmt, ...);
> -void tst_brkloopm(int ttype, void (*func)(void), char *arg_fmt, ...);
> +void tst_res(int ttype, char *fname, char *arg_fmt, ...)
> +	__attribute__ ((format (printf, 3, 4)));
> +void tst_resm(int ttype, char *arg_fmt, ...)
> +	__attribute__ ((format (printf, 2, 3)));
> +void tst_brk(int ttype, char *fname, void (*func)(void), char *arg_fmt, ...)
> +	__attribute__ ((format (printf, 4, 5)));
> +void tst_brkloop(int ttype, char *fname, void (*func)(void), char *arg_fmt, ...)
> +	__attribute__ ((format (printf, 4, 5)));
> +void tst_brkm(int ttype, void (*func)(void), char *arg_fmt, ...)
> +	__attribute__ ((format (printf, 3, 4)));
> +void tst_brkloopm(int ttype, void (*func)(void), char *arg_fmt, ...)
> +	__attribute__ ((format (printf, 3, 4)));
>  void tst_require_root(void (*func)(void));
>  int  tst_environ(void);
>  void tst_exit(void) LTP_ATTRIBUTE_NORETURN;


------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] Annotate tst_*() helpers with __attribute__ ((format (printf, M, N))) (was: Re: [PATCH] quotactl01: Fix tst_resm() format causing crash)

[Subrata Modak]

On Fri, 2009-08-28 at 11:38 +0530, Subrata Modak wrote:
> On Thu, 2009-08-27 at 17:29 +0200, Geert Uytterhoeven wrote: 
> > On Thu, 27 Aug 2009, Geert Uytterhoeven wrote:
> > > When the quotactl syscall fails, quotactl01 crashes with a segmentation fault
> > > due to an incorrect printf()-style format.
> > 
> > This bug encouraged me to add annotations to the test helpers that take
> > printf()-style formats, cfr. the patch below.
> > 
> > It causes a massive amount of compiler warnings, most of them caused by
> > TEST_ERRNO being long. According to CVS history, both TEST_RETURN and
> > TEST_ERRNO have been changed from int to long to accomodate 64-bit platforms,
> > but to me the change of TEST_ERRNO looks bogus.
> > As errno is int according to C99, TEST_ERRNO should actually be int too, right? 
> > 
> > Note that there are also a few other cases where integers are used on pointer
> > type format specifiers. These will cause crashes when the code path is
> > executed.
> > 
> > >From 601578f79c05779acefe023fd499726d9fe4ce03 Mon Sep 17 00:00:00 2001
> > From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> > Date: Thu, 27 Aug 2009 17:08:18 +0200
> > Subject: [PATCH] Annotate tst_*() helpers with __attribute__ ((format (printf, M, N)))
> > 
> > Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> 
> Ok. Thanks.

Sorry. I need to revert this as it generates a huge set of warning for
all the tests compiled. It should not only get fixed at test.h, but also
in all tests that uses the tst_* family of functions, which obviously is
too big to fix.

Regards--
Subrata

> 
> Regards--
> Subrata
> 
> > ---
> >  include/test.h |   18 ++++++++++++------
> >  1 files changed, 12 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/test.h b/include/test.h
> > index 34ed5d2..03b0d46 100644
> > --- a/include/test.h
> > +++ b/include/test.h
> > @@ -186,12 +186,18 @@
> >   * Functions from lib/tst_res.c
> >   */
> >  const char *strttype(int ttype);
> > -void tst_res(int ttype, char *fname, char *arg_fmt, ...);
> > -void tst_resm(int ttype, char *arg_fmt, ...);
> > -void tst_brk(int ttype, char *fname, void (*func)(void), char *arg_fmt, ...);
> > -void tst_brkloop(int ttype, char *fname, void (*func)(void), char *arg_fmt, ...);
> > -void tst_brkm(int ttype, void (*func)(void), char *arg_fmt, ...);
> > -void tst_brkloopm(int ttype, void (*func)(void), char *arg_fmt, ...);
> > +void tst_res(int ttype, char *fname, char *arg_fmt, ...)
> > +	__attribute__ ((format (printf, 3, 4)));
> > +void tst_resm(int ttype, char *arg_fmt, ...)
> > +	__attribute__ ((format (printf, 2, 3)));
> > +void tst_brk(int ttype, char *fname, void (*func)(void), char *arg_fmt, ...)
> > +	__attribute__ ((format (printf, 4, 5)));
> > +void tst_brkloop(int ttype, char *fname, void (*func)(void), char *arg_fmt, ...)
> > +	__attribute__ ((format (printf, 4, 5)));
> > +void tst_brkm(int ttype, void (*func)(void), char *arg_fmt, ...)
> > +	__attribute__ ((format (printf, 3, 4)));
> > +void tst_brkloopm(int ttype, void (*func)(void), char *arg_fmt, ...)
> > +	__attribute__ ((format (printf, 3, 4)));
> >  void tst_require_root(void (*func)(void));
> >  int  tst_environ(void);
> >  void tst_exit(void) LTP_ATTRIBUTE_NORETURN;
> 
> 
> ------------------------------------------------------------------------------
> Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
> trial. Simplify your report design, integration and deployment - and focus on 
> what you do best, core application coding. Discover what's new with 
> Crystal Reports now.  http://p.sf.net/sfu/bobj-july
> _______________________________________________
> Ltp-list mailing list
> Ltp-list@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ltp-list


------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] Annotate tst_*() helpers with __attribute__ ((format (printf, M, N))) (was: Re: [PATCH] quotactl01: Fix tst_resm() format causing crash)

[Geert Uytterhoeven]

On Fri, 28 Aug 2009, Subrata Modak wrote:
> On Fri, 2009-08-28 at 11:38 +0530, Subrata Modak wrote:
> > On Thu, 2009-08-27 at 17:29 +0200, Geert Uytterhoeven wrote: 
> > > On Thu, 27 Aug 2009, Geert Uytterhoeven wrote:
> > > > When the quotactl syscall fails, quotactl01 crashes with a segmentation fault
> > > > due to an incorrect printf()-style format.
> > > 
> > > This bug encouraged me to add annotations to the test helpers that take
> > > printf()-style formats, cfr. the patch below.
> > > 
> > > It causes a massive amount of compiler warnings, most of them caused by
> > > TEST_ERRNO being long. According to CVS history, both TEST_RETURN and
> > > TEST_ERRNO have been changed from int to long to accomodate 64-bit platforms,
> > > but to me the change of TEST_ERRNO looks bogus.
> > > As errno is int according to C99, TEST_ERRNO should actually be int too, right? 
> > > 
> > > Note that there are also a few other cases where integers are used on pointer
> > > type format specifiers. These will cause crashes when the code path is
> > > executed.
> > > 
> > > >From 601578f79c05779acefe023fd499726d9fe4ce03 Mon Sep 17 00:00:00 2001
> > > From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> > > Date: Thu, 27 Aug 2009 17:08:18 +0200
> > > Subject: [PATCH] Annotate tst_*() helpers with __attribute__ ((format (printf, M, N)))
> > > 
> > > Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> > 
> > Ok. Thanks.
> 
> Sorry. I need to revert this as it generates a huge set of warning for
> all the tests compiled. It should not only get fixed at test.h, but also
> in all tests that uses the tst_* family of functions, which obviously is
> too big to fix.

Yes, it should be fixed elsewhere, too. Reverting TEST_ERRNO to int should kill
a few thousand of them...

But without enabling the warnings, you don't notice the real bugs, where
integers are passed instead of string pointers.

With kind regards,

Geert Uytterhoeven
Software Architect
Techsoft Centre

Technology and Software Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] Annotate tst_*() helpers with __attribute__ ((format (printf, M, N)))

[Mike Frysinger]

[-- Attachment #1.1: Type: Text/Plain, Size: 1543 bytes --]

On Friday 28 August 2009 04:48:08 Geert Uytterhoeven wrote:
> On Fri, 28 Aug 2009, Subrata Modak wrote:
> > Sorry. I need to revert this as it generates a huge set of warning for
> > all the tests compiled. It should not only get fixed at test.h, but also
> > in all tests that uses the tst_* family of functions, which obviously is
> > too big to fix.
>
> Yes, it should be fixed elsewhere, too. Reverting TEST_ERRNO to int should
> kill a few thousand of them...

ive fixed that (i always thought it weird TEST_ERRNO was declared as a long).  
most of the remaining warnings are because TEST_RESULT is a long, but that is 
understandable.  we cram the result of arbitrary functions into it and we need 
something that is big enough.

that brings us down to ~900 warnings in testcases/kernel/syscalls/

> But without enabling the warnings, you don't notice the real bugs, where
> integers are passed instead of string pointers.

current warnings:
 - printf mismatches (i.e. long given to a %d)
 - NULL strings used for printf output
	- the tst_res code allows for this, but if you dont want output, then an 
empty string "" works just as well as a NULL string
 - too many arguments to printf string
 - too few arguments to printf string
 - incorrect printf string
 - some others

considering many of these printf's only get used when an error occurs (i.e. 
not normally executed), a crash is not likely to be noticed.  so doing this 
extra work of fixing the warnings is certainly worth it in my book.
-mike

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 355 bytes --]

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july

[-- Attachment #3: Type: text/plain, Size: 155 bytes --]

_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] Annotate tst_*() helpers with __attribute__ ((format (printf, M, N))) (was: Re: [PATCH] quotactl01: Fix tst_resm () format causing crash )

[Mike Frysinger]

[-- Attachment #1.1: Type: Text/Plain, Size: 1906 bytes --]

On Friday 28 August 2009 04:38:17 Subrata Modak wrote:
> On Fri, 2009-08-28 at 11:38 +0530, Subrata Modak wrote:
> > On Thu, 2009-08-27 at 17:29 +0200, Geert Uytterhoeven wrote:
> > > On Thu, 27 Aug 2009, Geert Uytterhoeven wrote:
> > > > When the quotactl syscall fails, quotactl01 crashes with a
> > > > segmentation fault due to an incorrect printf()-style format.
> > >
> > > This bug encouraged me to add annotations to the test helpers that take
> > > printf()-style formats, cfr. the patch below.
> > >
> > > It causes a massive amount of compiler warnings, most of them caused by
> > > TEST_ERRNO being long. According to CVS history, both TEST_RETURN and
> > > TEST_ERRNO have been changed from int to long to accomodate 64-bit
> > > platforms, but to me the change of TEST_ERRNO looks bogus.
> > > As errno is int according to C99, TEST_ERRNO should actually be int
> > > too, right?
> > >
> > > Note that there are also a few other cases where integers are used on
> > > pointer type format specifiers. These will cause crashes when the code
> > > path is executed.
> > >
> > > >From 601578f79c05779acefe023fd499726d9fe4ce03 Mon Sep 17 00:00:00 2001
> > >
> > > From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> > > Date: Thu, 27 Aug 2009 17:08:18 +0200
> > > Subject: [PATCH] Annotate tst_*() helpers with __attribute__ ((format
> > > (printf, M, N)))
> > >
> > > Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> >
> > Ok. Thanks.
>
> Sorry. I need to revert this as it generates a huge set of warning for
> all the tests compiled. It should not only get fixed at test.h, but also
> in all tests that uses the tst_* family of functions, which obviously is
> too big to fix.

nothing is too big.  the proposed patch is a great idea and anything 
preventing it from being merged should be resolved so it can go back in.
-mike

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 355 bytes --]

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july

[-- Attachment #3: Type: text/plain, Size: 155 bytes --]

_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] Annotate tst_*() helpers with __attribute__ ((format (printf, M, N))) (was: Re: [PATCH] quotactl01: Fix tst_resm () format causing crash )

[Subrata Modak]

On Fri, 2009-08-28 at 05:14 -0400, Mike Frysinger wrote:
> On Friday 28 August 2009 04:38:17 Subrata Modak wrote:
> > On Fri, 2009-08-28 at 11:38 +0530, Subrata Modak wrote:
> > > On Thu, 2009-08-27 at 17:29 +0200, Geert Uytterhoeven wrote:
> > > > On Thu, 27 Aug 2009, Geert Uytterhoeven wrote:
> > > > > When the quotactl syscall fails, quotactl01 crashes with a
> > > > > segmentation fault due to an incorrect printf()-style format.
> > > >
> > > > This bug encouraged me to add annotations to the test helpers that take
> > > > printf()-style formats, cfr. the patch below.
> > > >
> > > > It causes a massive amount of compiler warnings, most of them caused by
> > > > TEST_ERRNO being long. According to CVS history, both TEST_RETURN and
> > > > TEST_ERRNO have been changed from int to long to accomodate 64-bit
> > > > platforms, but to me the change of TEST_ERRNO looks bogus.
> > > > As errno is int according to C99, TEST_ERRNO should actually be int
> > > > too, right?
> > > >
> > > > Note that there are also a few other cases where integers are used on
> > > > pointer type format specifiers. These will cause crashes when the code
> > > > path is executed.
> > > >
> > > > >From 601578f79c05779acefe023fd499726d9fe4ce03 Mon Sep 17 00:00:00 2001
> > > >
> > > > From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> > > > Date: Thu, 27 Aug 2009 17:08:18 +0200
> > > > Subject: [PATCH] Annotate tst_*() helpers with __attribute__ ((format
> > > > (printf, M, N)))
> > > >
> > > > Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> > >
> > > Ok. Thanks.
> >
> > Sorry. I need to revert this as it generates a huge set of warning for
> > all the tests compiled. It should not only get fixed at test.h, but also
> > in all tests that uses the tst_* family of functions, which obviously is
> > too big to fix.
> 
> nothing is too big.  the proposed patch is a great idea and anything 
> preventing it from being merged should be resolved so it can go back in.

Great Mike. I am already seeing you doing some changes to the headers in
CVS. Can you also push this patch once you are done with all the
changes. Please notify me once all are done. I plan to checkout a fresh
one for the upcoming release ;-)

Regards--
Subrata

> -mike


------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] Annotate tst_*() helpers with __attribute__ ((format (printf, M, N)))

[Mike Frysinger]

[-- Attachment #1.1: Type: Text/Plain, Size: 1130 bytes --]

On Friday 28 August 2009 06:11:02 Subrata Modak wrote:
> On Fri, 2009-08-28 at 05:14 -0400, Mike Frysinger wrote:
> > On Friday 28 August 2009 04:38:17 Subrata Modak wrote:
> > > Sorry. I need to revert this as it generates a huge set of warning for
> > > all the tests compiled. It should not only get fixed at test.h, but
> > > also in all tests that uses the tst_* family of functions, which
> > > obviously is too big to fix.
> >
> > nothing is too big.  the proposed patch is a great idea and anything
> > preventing it from being merged should be resolved so it can go back in.
>
> Great Mike. I am already seeing you doing some changes to the headers in
> CVS. Can you also push this patch once you are done with all the
> changes. Please notify me once all are done. I plan to checkout a fresh
> one for the upcoming release ;-)

ive fixed a good chunk of the warnings now, but there's still a good number 
left.  maybe someone else feels like taking a crack at them, but i'm bored 
now.  i'll leave it up to you to apply said patch now or wait a few days so it 
goes in after the next release.
-mike

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 355 bytes --]

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july

[-- Attachment #3: Type: text/plain, Size: 155 bytes --]

_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] Annotate tst_*() helpers with?__attribute__ ((format (printf, M, N)))

[Cyril Hrubis]

Hi!
> > Great Mike. I am already seeing you doing some changes to the headers in
> > CVS. Can you also push this patch once you are done with all the
> > changes. Please notify me once all are done. I plan to checkout a fresh
> > one for the upcoming release ;-)
> 
> ive fixed a good chunk of the warnings now, but there's still a good number 
> left.  maybe someone else feels like taking a crack at them, but i'm bored 
> now.  i'll leave it up to you to apply said patch now or wait a few days so it 
> goes in after the next release.

Could you point me to the sources you have been working with. I'll try to have
look.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] Annotate tst_*() helpers with?__attribute__ ((format (printf, M, N)))

[Cyril Hrubis]

Hi!
> just apply the patch from Geert in this tread to latest CVS checkout and then 
> run `make`.  ive been looking at testcases/kernel/syscalls/, but i imagine 
> other dirs have new warnings now too.

Anybody knows what's the correct way of printing off_t in ltp? Google seems to
suggest either cast to intmax_t or some autoconf magick.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] Annotate tst_*() helpers with?__attribute__ ((format (printf, M, N)))

[Cyril Hrubis]

Hi!
>
> include inttypes.h and use PRId64
>

That's still gives warning for me:

warning: format '%lld' expects type 'long long int', but argument 6 has type 'off_t'

>
> i converted some tests to do this already (fallocate maybe?)

Hmm, looks like there is loff_t used instead of off_t.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] Annotate tst_*() helpers with?__attribute__ ((format (printf, M, N)))

[Cyril Hrubis]

Ahoj!
> > That's still gives warning for me:
> >
> > warning: format '%lld' expects type 'long long int', but argument 6 has
> > type 'off_t'
> >
> > > i converted some tests to do this already (fallocate maybe?)
> >
> > Hmm, looks like there is loff_t used instead of off_t.
> 
> loff_t is probably PRId64 while off_t is probably PRIu64

Hmm, seems like off_t is defined as long int on my machine. So following code
is came to my mind:

off_t t;
...
printf("off_t value %"PRId64"\n", (uint64_t) t);


-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] Annotate tst_*() helpers with __attribute__ ((format (printf, M, N)))

[Subrata Modak]

On Fri, 2009-08-28 at 10:44 -0400, Mike Frysinger wrote: 
> On Friday 28 August 2009 06:11:02 Subrata Modak wrote:
> > On Fri, 2009-08-28 at 05:14 -0400, Mike Frysinger wrote:
> > > On Friday 28 August 2009 04:38:17 Subrata Modak wrote:
> > > > Sorry. I need to revert this as it generates a huge set of warning for
> > > > all the tests compiled. It should not only get fixed at test.h, but
> > > > also in all tests that uses the tst_* family of functions, which
> > > > obviously is too big to fix.
> > >
> > > nothing is too big.  the proposed patch is a great idea and anything
> > > preventing it from being merged should be resolved so it can go back in.
> >
> > Great Mike. I am already seeing you doing some changes to the headers in
> > CVS. Can you also push this patch once you are done with all the
> > changes. Please notify me once all are done. I plan to checkout a fresh
> > one for the upcoming release ;-)
> 
> ive fixed a good chunk of the warnings now, but there's still a good number 

Thanks Mike.

> left.  maybe someone else feels like taking a crack at them, but i'm bored 
> now.  i'll leave it up to you to apply said patch now or wait a few days so it

I will do that right away.

Regards--
Subrata

> 
> goes in after the next release.
> -mike


------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] quotactl01: Fix tst_resm() format causing crash

[Mike Frysinger]

[-- Attachment #1.1: Type: Text/Plain, Size: 386 bytes --]

On Thursday 27 August 2009 11:04:40 Geert Uytterhoeven wrote:
> -                 	   	tst_resm(TFAIL, "%s failed - errno = %d : %s
> ...cmderror=%s", TCID, TEST_ERRNO, strerror(TEST_ERRNO),cmd[i]);
> +				tst_resm(TFAIL, "%s failed - errno = %d : %s ...cmd=0x%x", TCID,
> TEST_ERRNO, strerror(TEST_ERRNO), cmd[i]);

please use the new TTERRNO flag: tst_resm(TFAIL|TTERRNO, ....)
-mike

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 355 bytes --]

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july

[-- Attachment #3: Type: text/plain, Size: 155 bytes --]

_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list