[PATCH] xf86-input-hyperpen: Return proper default for unknown values in pInfo->device_control.

Peter Hutterer peter.hutterer at who-t.net
Mon Jul 18 16:20:16 PDT 2011


On Mon, Jul 18, 2011 at 11:44:21AM -0700, Terry Lambert wrote:
> On Sun, Jul 17, 2011 at 6:56 PM, Peter Hutterer
> <peter.hutterer at who-t.net> wrote:
> > On Fri, Jul 15, 2011 at 05:23:21PM -0700, Terry Lambert wrote:
> >> Signed-off-by: Terry Lambert <tlambert at chromium.org>
> >> Reviewed-by: Stephane Marchesin <marcheu at chromium.org>
> >> ---
> >>  src/xf86HyperPen.c |    3 +--
> >>  1 files changed, 1 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/src/xf86HyperPen.c b/src/xf86HyperPen.c
> >> index 45ddfca..b0e5ac9 100644
> >> --- a/src/xf86HyperPen.c
> >> +++ b/src/xf86HyperPen.c
> >> @@ -729,8 +729,7 @@ xf86HypProc(DeviceIntPtr pHyp, int what)
> >>
> >>      default:
> >>          ErrorF("unsupported mode=%d\n", what);
> >> -        return !Success;
> >> -        break;
> >> +     return BadValue;
> >>      }
> >>      DBG(2, ErrorF("END   xf86HypProc Success what=%d dev=%p priv=%p\n",
> >>                    what, (void *)pHyp, (void *)priv));
> >> --
> >> 1.7.3.1
> >
> > this one seems to be a bit unfinished. If I look at the DEVICE_INIT case,
> > there's plenty of times where !Success is returned. While this patch is
> > certainly correct, it seems that there is more stuff needed and I'd
> > appreciate the cleanup patches necessary here.
> >
> > Same applies for a bunch of other drivers. Anyway, I've pushed all of them
> > except the obsolete drivers. A few comments for the future though:
> > - please watch out for indentation. We don't use the same tab/spaces
> >  indentation rules in all drivers.
> > - use --subject-prefix for generating patches for other repositories, that
> >  way git am will skip it.
> >
> > Thanks
> >
> > Cheers,
> >  Peter
> 
> Thanks;
> 
> I've been called out in the past for making too many changes, so I was
> unsure of the proper balance, and was maybe overcautious.  I'll try to
> err on the side of fixing everything in the future, if that's OK in
> this venue. 8-).
> 
> I can look at the other !Success cases in those drivers inside a week,
> and see what would be appropriate as return codes in those cases.
> BadValue didn't look appropriate for most of them, so it might take
> some looking, and the driver maintainers might get to them before me
> (one can hope!).

Well, the problem with the return codes is that if we mix Success, !Success
and BadValue, the actual return value becomes a bit tricky to handle if it's
not well documented. Having defined error codes would be much easier but I
agree that it can be sometimes hard to find.  Looking at the hyperpen
driver, BadAlloc seems appropriate for all Init* call failures. I guess
this is just one more thing that should go into the next input ABI, I've
added it to my list now. Don't worry about it for now.

Cheers,
  Peter


More information about the xorg-devel mailing list