On Wed, Jun 15, 2011 at 10:35:19AM +0200, Alexander Graf wrote:
>
> On 14.06.2011, at 19:36, Michael S. Tsirkin wrote:
>
> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> > ---
> > linux-user/flatload.c | 10 ++++++----
> > linux-user/linuxload.c | 25 +------------------------
> > linux-user/main.c | 6 +++---
> > linux-user/signal.c | 5 -----
> > linux-user/syscall.c | 6 ------
> > 5 files changed, 10 insertions(+), 42 deletions(-)
> >
> > diff --git a/linux-user/flatload.c b/linux-user/flatload.c
> > index cd7af7c..2933c5f 100644
> > --- a/linux-user/flatload.c
> > +++ b/linux-user/flatload.c
> > @@ -379,12 +379,14 @@ static int load_flat_file(struct linux_binprm * bprm,
> > abi_long result;
> > abi_ulong realdatastart = 0;
> > abi_ulong text_len, data_len, bss_len, stack_len, flags;
> > - abi_ulong memp = 0; /* for finding the brk area */
> > abi_ulong extra;
> > abi_ulong reloc = 0, rp;
> > int i, rev, relocs = 0;
> > abi_ulong fpos;
> > - abi_ulong start_code, end_code;
> > + abi_ulong start_code;
> > +#ifdef DEBUG
> > + abi_ulong end_code;
> > +#endif
> > abi_ulong indx_len;
> >
> > hdr = ((struct flat_hdr *) bprm->buf); /* exec-header */
> > @@ -491,7 +493,6 @@ static int load_flat_file(struct linux_binprm * bprm,
> > }
> >
> > reloc = datapos + (ntohl(hdr->reloc_start) - text_len);
> > - memp = realdatastart;
> >
> > } else {
> >
> > @@ -506,7 +507,6 @@ static int load_flat_file(struct linux_binprm * bprm,
> > realdatastart = textpos + ntohl(hdr->data_start);
> > datapos = realdatastart + indx_len;
> > reloc = (textpos + ntohl(hdr->reloc_start) + indx_len);
> > - memp = textpos;
> >
> > #ifdef CONFIG_BINFMT_ZFLAT
> > #error code needs checking
> > @@ -552,7 +552,9 @@ static int load_flat_file(struct linux_binprm * bprm,
> >
> > /* The main program needs a little extra setup in the task structure */
> > start_code = textpos + sizeof (struct flat_hdr);
> > +#ifdef DEBUG
> > end_code = textpos + text_len;
> > +#endif
> >
> > DBG_FLT("%s %s: TEXT=%x-%x DATA=%x-%x BSS=%x-%x\n",
> > id ? "Lib" : "Load", bprm->filename,
> > diff --git a/linux-user/linuxload.c b/linux-user/linuxload.c
> > index ac8c486..62ebc7e 100644
> > --- a/linux-user/linuxload.c
> > +++ b/linux-user/linuxload.c
> > @@ -26,22 +26,6 @@ abi_long memcpy_to_target(abi_ulong dest, const void
> > *src,
> > return 0;
> > }
> >
> > -static int in_group_p(gid_t g)
> > -{
> > - /* return TRUE if we're in the specified group, FALSE otherwise */
> > - int ngroup;
> > - int i;
> > - gid_t grouplist[NGROUPS];
> > -
> > - ngroup = getgroups(NGROUPS, grouplist);
> > - for(i = 0; i < ngroup; i++) {
> > - if(grouplist[i] == g) {
> > - return 1;
> > - }
> > - }
> > - return 0;
> > -}
> > -
> > static int count(char ** vec)
> > {
> > int i;
> > @@ -57,7 +41,7 @@ static int prepare_binprm(struct linux_binprm *bprm)
> > {
> > struct stat st;
> > int mode;
> > - int retval, id_change;
> > + int retval;
> >
> > if(fstat(bprm->fd, &st) < 0) {
> > return(-errno);
> > @@ -73,14 +57,10 @@ static int prepare_binprm(struct linux_binprm *bprm)
> >
> > bprm->e_uid = geteuid();
> > bprm->e_gid = getegid();
> > - id_change = 0;
> >
> > /* Set-uid? */
> > if(mode & S_ISUID) {
> > bprm->e_uid = st.st_uid;
> > - if(bprm->e_uid != geteuid()) {
> > - id_change = 1;
> > - }
> > }
> >
> > /* Set-gid? */
> > @@ -91,9 +71,6 @@ static int prepare_binprm(struct linux_binprm *bprm)
> > */
> > if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) {
> > bprm->e_gid = st.st_gid;
> > - if (!in_group_p(bprm->e_gid)) {
> > - id_change = 1;
> > - }
> > }
> >
> > retval = read(bprm->fd, bprm->buf, BPRM_BUF_SIZE);
> > diff --git a/linux-user/main.c b/linux-user/main.c
> > index 04da0a4..9b995e5 100644
> > --- a/linux-user/main.c
> > +++ b/linux-user/main.c
> > @@ -2053,15 +2053,15 @@ void cpu_loop(CPUMIPSState *env)
> > } else {
> > int nb_args;
> > abi_ulong sp_reg;
> > - abi_ulong arg5 = 0, arg6 = 0, arg7 = 0, arg8 = 0;
> > + abi_ulong arg5 = 0, arg6 = 0;
> >
> > nb_args = mips_syscall_args[syscall_num];
> > sp_reg = env->active_tc.gpr[29];
> > switch (nb_args) {
> > /* these arguments are taken from the stack */
> > /* FIXME - what to do if get_user() fails? */
> > - case 8: get_user_ual(arg8, sp_reg + 28);
> > - case 7: get_user_ual(arg7, sp_reg + 24);
> > + case 8: /* get_user_ual(arg8, sp_reg + 28); */
> > + case 7: /* get_user_ual(arg7, sp_reg + 24); */
>
> I'd prefer to see these and the respective variable definitions #if 0'd with
> a comment, stating that they're currently unused.
>
> > case 6: get_user_ual(arg6, sp_reg + 20);
> > case 5: get_user_ual(arg5, sp_reg + 16);
> > default:
> > diff --git a/linux-user/signal.c b/linux-user/signal.c
> > index 11b25be..685ae61 100644
> > --- a/linux-user/signal.c
> > +++ b/linux-user/signal.c
> > @@ -2080,7 +2080,6 @@ long do_sigreturn(CPUState *env)
> > uint32_t up_psr, pc, npc;
> > target_sigset_t set;
> > sigset_t host_set;
> > - abi_ulong fpu_save_addr;
> > int err, i;
> >
> > sf_addr = env->regwptr[UREG_FP];
> > @@ -2120,8 +2119,6 @@ long do_sigreturn(CPUState *env)
> > err |= __get_user(env->regwptr[i + UREG_I0],
> > &sf->info.si_regs.u_regs[i+8]);
> > }
> >
> > - err |= __get_user(fpu_save_addr, &sf->fpu_save);
>
> This probably goes along the same line. Please keep the code around - it
> seems related to the commented out code below.
>
> > -
> > //if (fpu_save)
> > // err |= restore_fpu_state(env, fpu_save);
> >
> > @@ -2228,7 +2225,6 @@ void sparc64_set_context(CPUSPARCState *env)
> > target_mc_gregset_t *grp;
> > abi_ulong pc, npc, tstate;
> > abi_ulong fp, i7, w_addr;
> > - unsigned char fenab;
> > int err;
> > unsigned int i;
> >
> > @@ -2293,7 +2289,6 @@ void sparc64_set_context(CPUSPARCState *env)
> > if (put_user(i7, w_addr + offsetof(struct target_reg_window, ins[7]),
> > abi_ulong) != 0)
> > goto do_sigsegv;
> > - err |= __get_user(fenab, &(ucp->tuc_mcontext.mc_fpregs.mcfpu_enab));
>
> not sure here - might be the same as above?
>
> > err |= __get_user(env->fprs, &(ucp->tuc_mcontext.mc_fpregs.mcfpu_fprs));
> > {
> > uint32_t *src, *dst;
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index 5cb27c7..71395d5 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -3751,7 +3751,6 @@ static abi_long do_get_thread_area(CPUX86State *env,
> > abi_ulong ptr)
> > #ifndef TARGET_ABI32
> > static abi_long do_arch_prctl(CPUX86State *env, int code, abi_ulong addr)
> > {
> > - abi_long ret;
> > abi_ulong val;
> > int idx;
> >
> > @@ -3776,7 +3775,6 @@ static abi_long do_arch_prctl(CPUX86State *env, int
> > code, abi_ulong addr)
> > return -TARGET_EFAULT;
> > break;
> > default:
> > - ret = -TARGET_EINVAL;
>
> I'm fairly sure this was supposed to be "return -TARGET_EINVAL".
>
> > break;
> > }
> > return 0;
> > @@ -7058,18 +7056,14 @@ abi_long do_syscall(void *cpu_env, int num,
> > abi_long arg1,
> > case TARGET_NR_osf_sigprocmask:
> > {
> > abi_ulong mask;
> > - int how = arg1;
> > sigset_t set, oldset;
> >
> > switch(arg1) {
> > case TARGET_SIG_BLOCK:
> > - how = SIG_BLOCK;
> > break;
> > case TARGET_SIG_UNBLOCK:
> > - how = SIG_UNBLOCK;
> > break;
> > case TARGET_SIG_SETMASK:
> > - how = SIG_SETMASK;
>
> why go through the effort of setting "how" and then not using it? I'm pretty
> sure this is a bug as well. A few lines down is the following code:
>
> sigprocmask(arg1, &set, &oldset);
>
> which in TARGET_NR_sigprocmask would be:
>
> ret = get_errno(sigprocmask(how, &set, &oldset));
>
> So we end up sending guest masks to the host. Richard, this is Alpha specific
> code. Mind to double-check?
>
>
> Alex
Could you guys fix these the way you like?
I just want my build to pass ...
--
MST
