On Mon, Jul 18, 2011 at 11:44:21AM -0700, Terry Lambert wrote: > On Sun, Jul 17, 2011 at 6:56 PM, Peter Hutterer > <[email protected]> wrote: > > On Fri, Jul 15, 2011 at 05:23:21PM -0700, Terry Lambert wrote: > >> Signed-off-by: Terry Lambert <[email protected]> > >> Reviewed-by: Stephane Marchesin <[email protected]> > >> --- > >> 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 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
