https://bugs.kde.org/show_bug.cgi?id=506928

--- Comment #4 from [email protected] ---
Created attachment 183295
  --> https://bugs.kde.org/attachment.cgi?id=183295&action=edit
proposed patch

Hi Mark,

Thank you for your review.

(In reply to Mark Wielaard from comment #2)
> > +   if (ARG2 != 0) {
> > +      PRE_MEM_WRITE( "ustat(ubuf)", ARG2, sizeof(struct vki_ustat) );
> > +   }
> > +}
> 
> Don't need the ARG2 != 0.
> PRE_MEM_WRITE will warn if ARG2 is NULL, which is what we want.
> 
> > +POST(sys_ustat)
> > +{
> > +   if (ARG2 != 0) {
> > +      POST_MEM_WRITE( ARG1, sizeof(struct vki_ustat) );
> > +   }
> > +}
> 
> You don't need the ARG2 != 0. POST_MEM_WRITE should work on ARG2 (not ARG1).

In that case, would it make sense to do something like the following?

$ git diff
diff --git a/README_MISSING_SYSCALL_OR_IOCTL b/README_MISSING_SYSCALL_OR_IOCTL
index 8ddced5c9..d3aee517e 100644
--- a/README_MISSING_SYSCALL_OR_IOCTL
+++ b/README_MISSING_SYSCALL_OR_IOCTL
@@ -51,16 +51,12 @@ The wrapper for the time system call looks like this:
      /* time_t time(time_t *t); */
      PRINT("sys_time ( %p )",ARG1);
      PRE_REG_READ1(long, "time", int *, t);
-     if (ARG1 != 0) {
-        PRE_MEM_WRITE( "time(t)", ARG1, sizeof(vki_time_t) );
-     }
+     PRE_MEM_WRITE( "time(t)", ARG1, sizeof(vki_time_t) );
   }

   POST(sys_time)
   {  
-     if (ARG1 != 0) {
-        POST_MEM_WRITE( ARG1, sizeof(vki_time_t) );
-     }
+     POST_MEM_WRITE( ARG1, sizeof(vki_time_t) );
   }

 The first thing we do happens before the syscall occurs, in the PRE()
function.
$ 

> Please use #if defined(VGA_mips64), defined(VGA_mips32) and/or #elif
> defined(VGA_nanomips)
> depending on which Valgrind Architecure this holds for.
> 
> > +#ifdef __s390x__
> > +   unsigned int            f_tinode;
> > +#else
> > +   unsigned long           f_tinode;
> > +#endif
> 
> Please use #if defined(VGA_s390x)
> 
> > +   char                    f_fname[6];
> > +   char                    f_fpack[6];
> > +};
> > +

I did that.  For f_tfree, I know for sure it's correct for VGA_mips32 and
VGA_mips64.  I will  *assume* that the same applies to VGA_nanomips too.

I'm attaching updated patch.  I believe it does address all the points. 
Please, check.

-- 
You are receiving this mail because:
You are watching all bug changes.

Reply via email to