https://bugs.kde.org/show_bug.cgi?id=506928
--- Comment #5 from Mark Wielaard <[email protected]> --- Hi Martin, (In reply to mcermak from comment #4) > (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) ); > } Aha, it is the first example in README_MISSING_SYSCALL_OR_IOCTL. But there it is indeed necessary. time_t time(time_t *_Nullable tloc); If tloc == NULL then it just returns the time as the number of seconds since the Epoch. But if tloc != NULL then it also stores the time as time_t in the tloc buffer. Maybe we should make it more clear in the README that it depends on whether a pointer argument can be NULL that you should check for NULL before calling PRE_MEM_WRITE or POST_MEM_WRITE? If it isn't support to be NULL then PRE_MEM_WRITE will warn about it. > > 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. Yes, normally nonamips acts like mips32 > I'm attaching updated patch. I believe it does address all the points. > Please, check. Yes, looks good. Thanks. -- You are receiving this mail because: You are watching all bug changes.
