[PULL XTS] Misc fixes

Aaron Plattner aplattner at nvidia.com
Tue Mar 29 17:12:06 PDT 2011


Sorry for the very slow review.  Comments below.

On Sun, Nov 14, 2010 at 07:35:37PM -0800, Peter Hutterer wrote:
> Just found out that I had a whole bunch of fixes to XTS lying around on my
> branch. Any review would be appreciated.
> 
> The second-to-last commit is definitely debatable, should be refactured
> nicely instead of this way. But ETIME and whatnot.
> 
> Cheers,
>   Peter
> 
> The following changes since commit 00155fba2c650c86900a7a7ce68fefff10dce3de:
> 
>   xts5: Bail out with UNTESTED for BAD_LENGTH requests that would have length 0 (2010-10-18 16:16:03 -0700)
> 
> are available in the git repository at:
>   git://people.freedesktop.org/~whot/xts.git master
> 
> Peter Hutterer (21):
>       Remove superfluous (void) casts.
>       Remove EXPECT_NOTHING pointer to int cast.
>       Purge wbzero - use memset instead.

These three Reviewed-by: Aaron Plattner <aplattner at nvidia.com>

>       purge wbcopy, replace with memcpy

wbcopy implementes memmove, not memcpy.  In partcular, this doesn't seem
safe:

-	wbcopy((char *)valuePtr,(char *)(valuePtr+1),(after<<2));
+	memcpy((char *)(valuePtr+1), (char *)valuePtr, (after<<2));

>       Purge wbcmp - unused. Use memcmp instead anyway
>       Purge wffs - use ffs(3) instead.
>       Purge unused Get_Date().
>       Remove now empty Utils.c

Reviewed-by: Aaron Plattner <aplattner at nvidia.com>

>       tet3: if TET_RESFILE is not in the env, guess it from the program name.

Whitespace doesn't match surrounding code.

>       tpstartup needs to reset config.alt_screens for single-display setups.

It looks like alt_screen comes from XT_ALT_SCREEN in the config, which is
set to UNSUPPORTED (which translates to -1) if XT_SCREEN_COUNT is 1.
There's lots of other stuff that will cause test failures if your config
was generated against a different server so this band-aid doesn't seem
appropriate.

If a test is fiddling with alt_screen during the run, then that seems like
a test bug.

>       tcm: move envmsg up into TET_LITE ifndef
>       Fix one more out-of-tree build error.
>       xts5: fix compiler warning.
>       Fix a bunch of compiler warnings for default return type.
>       Fix compiler warning - return missing for non-void function.     ../../../../xts5/src/libproto/DfltVals.c: In function ‘Gen_Good_Event’:     ../../../../xts5/src/libproto/DfltVals.c:395: warning: control reaches end of non-void function
>       Purge a bunch of unused Gen_Good_foo()

The above,
Reviewed-by: Aaron Plattner <aplattner at nvidia.com>

What is Gen_Good_Event used for?   I don't see it used anywhere.

>       more void cast removal

I'll take your word for it.  :)

Acked-by: Aaron Plattner <aplattner at nvidia.com>

>       My NULL is bigger than yours, don't you dare redefine it.
>       mc: Make main() return an int instead of calling exit(0);

The above,
Reviewed-by: Aaron Plattner <aplattner at nvidia.com>

>       Fix a few "return type defaults to 'int' compiler warnings.

Let's make these return void where appropriate.  I don't think anyone's in
a huge rush to ship xts-1.0.

>       Xlib13: fix XSetModfierMapping test 1/8.

s/Modfier/Modifier/g in the description.  Also, you might want to quote the
spec:

    "The order of keycodes within each set is chosen arbitrarily by the server."

It seems like this check is also bogus:

 	if (newmap->max_keypermod == modmap->max_keypermod)
 		CHECK;
 	else {
 		report("max_keypermod was %d, expecting %d", newmap->max_keypermod,
 			modmap->max_keypermod);
 		FAIL;
 	}

because, "The keycodes-per-modifier value is chosen arbitrarily by the
server".  It seems like the test needs to be able to handle an output kpm
different from the input one.

-- Aaron


More information about the xorg-devel mailing list