valgrind-users

[Valgrind-users] [PATCH] JSIOCGNAME support

[Melchior F. <mf...@kd...>]

The JSIOCGNAME ioctl is currently not supported by valgrind. This function
returns human readable joystick names (like "SAITEK CYBORG 3D USB"). The ioctl
has two unconventional properties: it has a variable id (with the size of
the target buffer encoded in it), and it doesn't necessarily return zero
on success, but the number of actually written bytes. I suggest the following
patch.

m.



Index: coregrind/vg_syscalls.c
===================================================================
RCS file: /cvsroot/valgrind/valgrind/coregrind/vg_syscalls.c,v
retrieving revision 1.32
diff -u -p -r1.32 vg_syscalls.c
--- coregrind/vg_syscalls.c	2 May 2003 16:18:06 -0000	1.32
+++ coregrind/vg_syscalls.c	2 May 2003 16:24:03 -0000
@@ -2435,9 +2435,18 @@ void VG_(perform_assumed_nonblocking_sys
                }
                KERNEL_DO_SYSCALL(tid,res);
                if (size > 0 && (dir & _IOC_READ)
-                   && !VG_(is_kerror)(res) && res == 0
-                   && arg3 != (Addr)NULL)
-                  VG_TRACK( post_mem_write,arg3, size);
+                   && !VG_(is_kerror)(res)
+                   && arg3 != (Addr)NULL) {
+                  /* JSIOCGNAME() returns the number of actually
+                   * written bytes on success.
+                   */
+                  if ((arg2 & ~(_IOC_SIZEMASK << _IOC_SIZESHIFT))
+                      == JSIOCGNAME(0)) {
+                     if (res > 0)
+                        VG_TRACK( post_mem_write, arg3, res);
+                  } else if (res == 0)
+                     VG_TRACK( post_mem_write, arg3, size);
+               }
                break;
             }
          }
Index: coregrind/vg_unsafe.h
===================================================================
RCS file: /cvsroot/valgrind/valgrind/coregrind/vg_unsafe.h,v
retrieving revision 1.12
diff -u -p -r1.12 vg_unsafe.h
--- coregrind/vg_unsafe.h	15 Apr 2003 14:57:51 -0000	1.12
+++ coregrind/vg_unsafe.h	2 May 2003 16:24:04 -0000
@@ -60,8 +60,9 @@
 #include <sys/stat.h>     /* for struct stat */
 #undef __USE_LARGEFILE64
 
-#include <asm/ioctls.h>   /* for stuff for dealing with ioctl :( */
-#include <sys/soundcard.h> /* for various soundcard ioctl constants :( */
+#include <asm/ioctls.h>     /* for stuff for dealing with ioctl :( */
+#include <linux/joystick.h> /* for JSIOCGNAME joystick ioctl :( */
+#include <sys/soundcard.h>  /* for various soundcard ioctl constants :( */
 
 #ifndef GLIBC_2_1
 #  include <linux/rtc.h>   /* for RTC_* ioctls */

[Valgrind-users] Re: [PATCH] JSIOCGNAME support [anniversary]

[Melchior F. <mf...@kd...>]

* Melchior FRANZ -- Friday 02 May 2003 18:33:
> [...] I suggest the following patch.

This patch has now been ignored for exactly one month. Do I have
to consider it rejected? Because it deals with joysticks, which is
children stuff and not taken seriously? Other reasons?   :-)

m.

Re: [Valgrind-users] Re: [PATCH] JSIOCGNAME support [anniversary]

[Nicholas N. <nj...@ca...>]

On Mon, 2 Jun 2003, Melchior FRANZ wrote:

> * Melchior FRANZ -- Friday 02 May 2003 18:33:
> > [...] I suggest the following patch.
>
> This patch has now been ignored for exactly one month. Do I have
> to consider it rejected? Because it deals with joysticks, which is
> children stuff and not taken seriously? Other reasons?   :-)

Other reasons?  Lack of time.

We haven't integrated/addressed your patch, but that doesn't mean we
have ignored it.  And don't think you're alone in having contributed a
patch that hasn't been integrated...

Julian Seward is Valgrind's main developer.  He spent two years writing
Valgrind in his spare time.  When it was first released, he took 6 months
off work to develop it full time, unpaid.  Valgrind's robustness can be
largely attributed to the effort he put in chasing down bugs in this time.
He now has a full-time non-Valgrind job, and thus has limited time to work
on Valgrind.  He is the expert on Valgrind's synthetic CPU, and all the
OS-specific stuff, such as threads, signals, system calls, etc.  The
OS-specific stuff happens to be the most fragile and bug-prone part of
Valgrind, so Julian spends a lot of his (limited) Valgrind time on fixing
bugs in these parts.  He also spends a lot of time testing on various
Linux configurations, and managing releases.

I guess I'd call myself the second developer.  I'm a PhD student, and
joined the show when Valgrind was first released, a bit over one year ago.
My PhD is based around using Valgrind, which means I have a bit more time
to spend on it for maintenance, but primarily I'm using it as a vehicle
for research;  time I spend maintaining it is time I'm not doing research.
I try to fix bugs, make improvements, etc. that people submit, but my area
of expertise is the core/skin split and instrumentation side, plus
Cachegrind.  I try to fix as many bugs as I can, but many of them are
beyond my area of expertise.

Some other people have contributed significant amounts to Valgrind, such
as Jeremy Fitzhardinge (Helgrind, lots of other stuff), Josef Weidendorfer
(KCachegrind, other core stuff), Dirk Mueller, and probably lots of others
I haven't mentioned.  AIUI, for these people Valgrind is something that
they can't spend huge amounts of time on.

Another point worth mentioning is that Valgrind is a fairly complicated
piece of software, now beyond what one person can keep in his/her head,
and it has very complex interactions with your system that mean it is
fragile in many ways.  Thus we have to be careful when making changes,
particularly ones related to operating system stuff, such as your patch.

I currently have 40 emails in my inbox, 36 of which are Valgrind related,
being bug reports, patches, suggestions for features, and questions.  I
also have tens of other similar emails which are stagnating in other
folders.  Julian probably has more.  The problem is time, or lack of it.

The reason Julian started this list was to try and improve the feedback
users were getting, since he and I can't deal with all the mail we get.
We have an implicit triage system;  if something is reported only once, we
will probably address it if it is bad, and can be addressed without
massive amounts of effort.  Also, if something is mentioned more than
once, it's much more likely to be addressed.  For example, Julian
implemented support for leak suppressions because lots of people requested
it.  I implemented support for automatic suppression generation (not in
1.9.6, but in the CVS HEAD) because lots of people asked for it.  Julian
worked quite hard to get Valgrind working with RH9 to handle the new
thread library because lots of people sent bug reports.

Your patch is small, you're the only one who has mentioned it, and you've
obviously worked out how to fix it yourself.  This makes it a fairly low
priority.  You also submitted it just before the 1.9.6 release came out,
too late to include in that release.  No release has come out since.  It's
possible it will be included in 1.9.7.  The fact that you sent a patch,
rather than just a bug report, is a big help too;  it makes our lives much
easier, so thank you for that.  The fact that it is small and simple also
helps.

I don't wish to complain, it's great that so many people are using
Valgrind, reporting bugs, submitting patches, making suggestions.  I can
only speak for myself:  I read and consider every Valgrind-related email
that I receive, but I don't/can't respond to every one, due to lack of
time and/or expertise.  Julian has more expertise than me, but less time.
Please appreciate the time constraints that face us.

N

Re: [Valgrind-users] Re: [PATCH] JSIOCGNAME support [anniversary]

[Julian S. <js...@ac...>]

On Monday 02 June 2003 09:35, Melchior FRANZ wrote:
> * Melchior FRANZ -- Friday 02 May 2003 18:33:
> > [...] I suggest the following patch.
>
> This patch has now been ignored for exactly one month. Do I have
> to consider it rejected? Because it deals with joysticks, which is
> children stuff and not taken seriously? Other reasons?   :-)

Sorry, should have replied to this earlier.  As I remember, this patch
requires #including a new file into coregrind/vg_unsafe.h, and 
I am pretty paranoid about doing that because in the past such
patches have caused compilation failures on some Linux distros
and not others.  But you're right, I should have said something
earlier about it.

J

[Valgrind-users] Re: [PATCH] JSIOCGNAME support [anniversary]

[Melchior F. <mf...@kd...>]

* Julian Seward -- Sunday 06 July 2003 14:57:
> As I remember, this patch
> requires #including a new file into coregrind/vg_unsafe.h, and 
> I am pretty paranoid about doing that because in the past such
> patches have caused compilation failures on some Linux distros
> and not others.

Well, that's of course a problem. But would it decide whether some
Linux ioctl will be supported or not? ("We don't support new ioctls,
if this means to include yet another Linux kernel header." ?)
As of now, valgrind false-positively reports "use of uninitialized
memory" and it took me hours to find out that the bug was actually
in valgrind, not in FlightGear. (I could have found out faster, but
my trust in valgrind was unlimited. ;-) It would be nice if others
wouldn't have to repeat this experience. It's not for =me=, because
I =do= have this fixed in my version.

m.  :-)