Re: [PATCH] Factor out printing of 64bit syscall argument
On Wed, Nov 04, 2009 at 05:20:42PM +0100, Andreas Schwab wrote: > Here is an updated patch. It looks OK for me now. -- ldv pgpu0vGGdmkfM.pgp Description: PGP signature -- 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___ Strace-devel mailing list Strace-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/strace-devel
Re: [PATCH] Factor out printing of 64bit syscall argument
On Wed, Nov 04, 2009 at 05:01:31PM +0100, Andreas Schwab wrote:
> "Dmitry V. Levin" writes:
> > On Wed, Nov 04, 2009 at 12:55:46PM +0100, Andreas Schwab wrote:
> >> This patch refactors the printing of 64bit syscall arguments.
> >
> > I like the change, but I have two questions about particular hunks of
> > the patch.
> >
> >> @@ -631,7 +625,6 @@ sys_lseek64(struct tcb *tcp)
> >> {
> >>if (entering(tcp)) {
> >>long long offset;
> >> - ALIGN64 (tcp, 1); /* FreeBSD aligns off_t args */
> >>offset = LONG_LONG(tcp->u_arg [1], tcp->u_arg[2]);
> >>if (tcp->u_arg[3] == SEEK_SET)
> >>tprintf("%ld, %llu, ", tcp->u_arg[0], offset);
> >
> > Why printllval() is not used here?
>
> Accidentally dropped the ball here.
I expect that LONG_LONG macro should no longer be used beyond the
printllval() definition, except some dead ifdef'ed code in io.c.
> >> -#elif defined ARM || defined POWERPC
> >> - LONG_LONG(tcp->u_arg[2], tcp->u_arg[3]),
> >> - LONG_LONG(tcp->u_arg[4], tcp->u_arg[5]));
> >> + int argn;
> >> + tprintf("%ld, ", tcp->u_arg[0]);
> >> + argn = printllval(tcp, "%lld, ", 1);
> >> + argn = printllval(tcp, "%lld, ", argn);
> >> +#if defined ARM || defined POWERPC
> >>printxval(advise, tcp->u_arg[1], "POSIX_FADV_???");
> >> #else
> >> - LONG_LONG(tcp->u_arg[1], tcp->u_arg[2]),
> >> - LONG_LONG(tcp->u_arg[3], tcp->u_arg[4]));
> >> - printxval(advise, tcp->u_arg[5], "POSIX_FADV_???");
> >> + printxval(advise, tcp->u_arg[argn], "POSIX_FADV_???");
> >> #endif
> >>}
> >>return 0;
> >
> > I'm afraid that this change breaks ARM because printxval() does no
> > special handling for ARM.
>
> I guess you mean printllval? Yes, that breaks ARM.
Yes, I meant printllval().
--
ldv
pgpEOqJnsxSfB.pgp
Description: PGP signature
--
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___
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel
Re: [PATCH] Factor out printing of 64bit syscall argument
Here is an updated patch.
Andreas.
>From b5600fc3df0453ba11f254a9b49add3ffbec9733 Mon Sep 17 00:00:00 2001
From: Andreas Schwab
Date: Wed, 4 Nov 2009 17:08:34 +0100
Subject: [PATCH] Factor out printing of 64bit syscall argument
* defs.h (ALIGN64): Remove.
(printllval): Declare.
* util.c (printllval): Define.
* file.c (sys_readahead): Use printllval.
(sys_lseek64): Likewise.
(sys_truncate64): Likewise.
(sys_ftruncate64): Likewise.
(sys_fadvise64): Likewise.
(sys_fadvise64_64): Likewise.
(sys_fallocate): Likewise.
* io.c (sys_pread): Likewise.
(sys_pwrite): Likewise.
(sys_pread64): Likewise.
(sys_pwrite64): Likewise.
* mem.c (sys_mmap64): Likewise.
---
defs.h | 16 +---
file.c | 79 ---
io.c | 28 ++
mem.c |3 +-
util.c | 36 +
5 files changed, 77 insertions(+), 85 deletions(-)
diff --git a/defs.h b/defs.h
index 5bcaa07..dc0d91f 100644
--- a/defs.h
+++ b/defs.h
@@ -651,20 +651,6 @@ extern const char *const signalent2[];
extern const int nsignals2;
#endif /* SUPPORTED_PERSONALITIES >= 3 */
-#if defined(FREEBSD) || (defined(LINUX) \
-&& defined(POWERPC) && !defined(__powerpc64__)) \
- || defined (LINUX_MIPSO32)
-/* ARRGH! off_t args are aligned on 64 bit boundaries! */
-#define ALIGN64(tcp,arg) \
-do { \
- if (arg % 2)\
- memmove (&tcp->u_arg[arg], &tcp->u_arg[arg + 1],\
-(tcp->u_nargs - arg - 1) * sizeof tcp->u_arg[0]); \
-} while (0)
-#else
-#define ALIGN64(tcp,arg) do { } while (0)
-#endif
-
#if HAVE_LONG_LONG
/* _l refers to the lower numbered u_arg,
@@ -678,6 +664,8 @@ do {
\
#define LONG_LONG(_l,_h) \
((long long)((unsigned long long)(unsigned)(_h) | ((unsigned long
long)(_l)<<32)))
#endif
+
+extern int printllval(struct tcb *, const char *, int);
#endif
#ifdef IA64
diff --git a/file.c b/file.c
index 2e46284..14df971 100644
--- a/file.c
+++ b/file.c
@@ -610,16 +610,10 @@ int
sys_readahead(struct tcb *tcp)
{
if (entering(tcp)) {
- ALIGN64 (tcp, 1);
- tprintf("%ld, %lld, %ld", tcp->u_arg[0],
-# if defined LINUX_MIPSN32
- tcp->ext_arg[1], tcp->u_arg[2]
-# elif defined IA64 || defined X86_64 || defined ALPHA || defined
LINUX_MIPSN64 || (defined POWERPC && defined __powerpc64__)
- (long long int) tcp->u_arg[1], tcp->u_arg[2]
-# else
- LONG_LONG(tcp->u_arg[1], tcp->u_arg[2]), tcp->u_arg[3]
-# endif
- );
+ int argn;
+ tprintf("%ld, ", tcp->u_arg[0]);
+ argn = printllval(tcp, "%lld", 1);
+ tprintf(", %ld", tcp->u_arg[argn]);
}
return 0;
}
@@ -630,14 +624,13 @@ int
sys_lseek64(struct tcb *tcp)
{
if (entering(tcp)) {
- long long offset;
- ALIGN64 (tcp, 1); /* FreeBSD aligns off_t args */
- offset = LONG_LONG(tcp->u_arg [1], tcp->u_arg[2]);
+ int argn;
+ tprintf("%ld, ", tcp->u_arg[0]);
if (tcp->u_arg[3] == SEEK_SET)
- tprintf("%ld, %llu, ", tcp->u_arg[0], offset);
+ argn = printllval(tcp, "%llu, ", 1);
else
- tprintf("%ld, %lld, ", tcp->u_arg[0], offset);
- printxval(whence, tcp->u_arg[3], "SEEK_???");
+ argn = printllval(tcp, "%lld, ", 1);
+ printxval(whence, tcp->u_arg[argn], "SEEK_???");
}
return RVAL_LUDECIMAL;
}
@@ -660,9 +653,8 @@ int
sys_truncate64(struct tcb *tcp)
{
if (entering(tcp)) {
- ALIGN64 (tcp, 1);
printpath(tcp, tcp->u_arg[0]);
- tprintf(", %llu", LONG_LONG(tcp->u_arg[1],tcp->u_arg[2]));
+ printllval(tcp, ", %llu", 1);
}
return 0;
}
@@ -684,9 +676,8 @@ int
sys_ftruncate64(struct tcb *tcp)
{
if (entering(tcp)) {
- ALIGN64 (tcp, 1);
- tprintf("%ld, %llu", tcp->u_arg[0],
- LONG_LONG(tcp->u_arg[1] ,tcp->u_arg[2]));
+ tprintf("%ld, ", tcp->u_arg[0]);
+ printllval(tcp, "%llu", 1);
}
return 0;
}
@@ -2816,16 +2807,11 @@ int
sys_fadvise64(struct tcb *tcp)
{
if (entering(tcp)) {
- ALIGN64(tcp, 1);
- tprintf("%ld, %lld, %ld, ",
- tcp->u_arg[0],
-# if defined IA64 || defined X86_64 || defined ALPHA || (defined POWERPC &&
defined __powerpc64__)
- (long long int) tcp->u_arg[1], tcp->u_arg[2]);
- printxval(advise, tcp->u_arg[3], "POSIX_F
Re: [PATCH] Factor out printing of 64bit syscall argument
"Dmitry V. Levin" writes:
> On Wed, Nov 04, 2009 at 12:55:46PM +0100, Andreas Schwab wrote:
>> This patch refactors the printing of 64bit syscall arguments.
>
> I like the change, but I have two questions about particular hunks of
> the patch.
>
>> @@ -631,7 +625,6 @@ sys_lseek64(struct tcb *tcp)
>> {
>> if (entering(tcp)) {
>> long long offset;
>> -ALIGN64 (tcp, 1); /* FreeBSD aligns off_t args */
>> offset = LONG_LONG(tcp->u_arg [1], tcp->u_arg[2]);
>> if (tcp->u_arg[3] == SEEK_SET)
>> tprintf("%ld, %llu, ", tcp->u_arg[0], offset);
>
> Why printllval() is not used here?
Accidentally dropped the ball here.
>> @@ -2836,22 +2822,14 @@ int
>> sys_fadvise64_64(struct tcb *tcp)
>> {
>> if (entering(tcp)) {
>> -tprintf("%ld, %lld, %lld, ",
>> -tcp->u_arg[0],
>> -#if defined LINUX_MIPSN32
>> -tcp->ext_arg[1], tcp->ext_arg[2]);
>> -printxval(advise, tcp->u_arg[3], "POSIX_FADV_???");
>> -#elif defined IA64 || defined X86_64 || defined ALPHA || defined
>> LINUX_MIPSN64
>> -(long long int) tcp->u_arg[1], (long long int)
>> tcp->u_arg[2]);
>> -printxval(advise, tcp->u_arg[3], "POSIX_FADV_???");
>> -#elif defined ARM || defined POWERPC
>> -LONG_LONG(tcp->u_arg[2], tcp->u_arg[3]),
>> -LONG_LONG(tcp->u_arg[4], tcp->u_arg[5]));
>> +int argn;
>> +tprintf("%ld, ", tcp->u_arg[0]);
>> +argn = printllval(tcp, "%lld, ", 1);
>> +argn = printllval(tcp, "%lld, ", argn);
>> +#if defined ARM || defined POWERPC
>> printxval(advise, tcp->u_arg[1], "POSIX_FADV_???");
>> #else
>> -LONG_LONG(tcp->u_arg[1], tcp->u_arg[2]),
>> -LONG_LONG(tcp->u_arg[3], tcp->u_arg[4]));
>> -printxval(advise, tcp->u_arg[5], "POSIX_FADV_???");
>> +printxval(advise, tcp->u_arg[argn], "POSIX_FADV_???");
>> #endif
>> }
>> return 0;
>
> I'm afraid that this change breaks ARM because printxval() does no
> special handling for ARM.
I guess you mean printllval? Yes, that breaks ARM.
Andreas.
--
Andreas Schwab, sch...@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84 5EC7 45C6 250E 6F00 984E
"And now for something completely different."
--
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
___
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel
Re: [PATCH] Factor out printing of 64bit syscall argument
On Wed, Nov 04, 2009 at 12:55:46PM +0100, Andreas Schwab wrote:
> This patch refactors the printing of 64bit syscall arguments.
I like the change, but I have two questions about particular hunks of
the patch.
> @@ -631,7 +625,6 @@ sys_lseek64(struct tcb *tcp)
> {
> if (entering(tcp)) {
> long long offset;
> - ALIGN64 (tcp, 1); /* FreeBSD aligns off_t args */
> offset = LONG_LONG(tcp->u_arg [1], tcp->u_arg[2]);
> if (tcp->u_arg[3] == SEEK_SET)
> tprintf("%ld, %llu, ", tcp->u_arg[0], offset);
Why printllval() is not used here?
> @@ -2836,22 +2822,14 @@ int
> sys_fadvise64_64(struct tcb *tcp)
> {
> if (entering(tcp)) {
> - tprintf("%ld, %lld, %lld, ",
> - tcp->u_arg[0],
> -#if defined LINUX_MIPSN32
> - tcp->ext_arg[1], tcp->ext_arg[2]);
> - printxval(advise, tcp->u_arg[3], "POSIX_FADV_???");
> -#elif defined IA64 || defined X86_64 || defined ALPHA || defined
> LINUX_MIPSN64
> - (long long int) tcp->u_arg[1], (long long int)
> tcp->u_arg[2]);
> - printxval(advise, tcp->u_arg[3], "POSIX_FADV_???");
> -#elif defined ARM || defined POWERPC
> - LONG_LONG(tcp->u_arg[2], tcp->u_arg[3]),
> - LONG_LONG(tcp->u_arg[4], tcp->u_arg[5]));
> + int argn;
> + tprintf("%ld, ", tcp->u_arg[0]);
> + argn = printllval(tcp, "%lld, ", 1);
> + argn = printllval(tcp, "%lld, ", argn);
> +#if defined ARM || defined POWERPC
> printxval(advise, tcp->u_arg[1], "POSIX_FADV_???");
> #else
> - LONG_LONG(tcp->u_arg[1], tcp->u_arg[2]),
> - LONG_LONG(tcp->u_arg[3], tcp->u_arg[4]));
> - printxval(advise, tcp->u_arg[5], "POSIX_FADV_???");
> + printxval(advise, tcp->u_arg[argn], "POSIX_FADV_???");
> #endif
> }
> return 0;
I'm afraid that this change breaks ARM because printxval() does no
special handling for ARM.
--
ldv
pgpRSRzhiMmz1.pgp
Description: PGP signature
--
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___
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel
