On Tue, 2011-08-23 at 15:00 +0400, Dmitry V. Levin wrote:
> I agree, but first I'd like to reduce redundancy in syscall_enter().
> I have a not quite tested patch, please have a look:
>
> diff --git a/syscall.c b/syscall.c
> index 2fd5bb4..7beb0cb 100644
> --- a/syscall.c
> +++ b/syscall.c
> @@ -1949,22 +1949,20 @@ static int
> syscall_enter(struct tcb *tcp)
> {
> #ifdef LINUX
> -# if defined(S390) || defined(S390X)
> int i;
Cosmetics: on some arches which do not use i because they have no arg
reading loop, you might get a warning about unused i.
> - if (tcp->scno >= 0 && tcp->scno < nsyscalls && sysent[tcp->scno].nargs
> != -1)
> +
> + if (tcp->scno >= 0 && tcp->scno < nsyscalls &&
> + sysent[tcp->scno].nargs >= 0 && sysent[tcp->scno].nargs <= MAX_ARGS)
The "sysent[tcp->scno].nargs >= 0 && sysent[tcp->scno].nargs <=
MAX_ARGS" condition should be always true. Why waste cycles testing it?
If you feel we need to safeguard against bugs in the table,
do the test just once for the entire table(s) at stracr startup.
gcc is stupid and "tcp->scno >= 0 && tcp->scno < nsyscalls"
is done as literally two comparisons. Maybe use
more efficient "(unsigned long)tcp->scno < nsyscalls".
More intrusive alternative is to declare tcp->scno member as *unsigned*
long, but then... what a hell is * this* in svr4/syscallent.h???
#ifdef MIPS
{ -1, 0, printargs, "SYS_-1" }, /* -1 */
#endif /* MIPS */
scno == -1 is **VALID** on mips???
> tcp->u_nargs = sysent[tcp->scno].nargs;
> else
> tcp->u_nargs = MAX_ARGS;
> +
> +# if defined(S390) || defined(S390X)
> for (i = 0; i < tcp->u_nargs; i++) {
> if (upeek(tcp, i==0 ? PT_ORIGGPR2 : PT_GPR2 + i*sizeof(long),
> &tcp->u_arg[i]) < 0)
> return -1;
> }
> # elif defined(ALPHA)
> - int i;
> - if (tcp->scno >= 0 && tcp->scno < nsyscalls && sysent[tcp->scno].nargs
> != -1)
> - tcp->u_nargs = sysent[tcp->scno].nargs;
> - else
> - tcp->u_nargs = MAX_ARGS;
> for (i = 0; i < tcp->u_nargs; i++) {
> /* WTA: if scno is out-of-bounds this will bomb. Add range-check
> * for scno somewhere above here!
> @@ -1974,7 +1972,7 @@ syscall_enter(struct tcb *tcp)
> }
> # elif defined(IA64)
> if (!ia32) {
> - unsigned long *out0, cfm, sof, sol, i;
> + unsigned long *out0, cfm, sof, sol;
> long rbs_end;
> /* be backwards compatible with kernel < 2.4.4... */
> # ifndef PT_RBS_END
> @@ -1990,80 +1988,53 @@ syscall_enter(struct tcb *tcp)
> sol = (cfm >> 7) & 0x7f;
> out0 = ia64_rse_skip_regs((unsigned long *) rbs_end, -sof +
> sol);
>
> - if (tcp->scno >= 0 && tcp->scno < nsyscalls
> - && sysent[tcp->scno].nargs != -1)
> - tcp->u_nargs = sysent[tcp->scno].nargs;
> - else
> - tcp->u_nargs = MAX_ARGS;
...
...
...
The rest looks good, nothing to nitpick at.
--
vda
------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system,
user administration capabilities and model configuration. Take
the hassle out of deploying and managing Subversion and the
tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel
