[PATCH 1/2] Add _X_UNUSED attribute to designate unused variables and silence warnings

2011-04-27 Thread Jeremy Huddleston

Signed-off-by: Jeremy Huddleston 
---

Alan, what SunCC magic needs to go here?

 Xfuncproto.h.in |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/Xfuncproto.h.in b/Xfuncproto.h.in
index 2b8a226..88d91c0 100644
--- a/Xfuncproto.h.in
+++ b/Xfuncproto.h.in
@@ -122,6 +122,12 @@ in this Software without prior written authorization from 
The Open Group.
 # define _X_ATTRIBUTE_PRINTF(x,y)
 #endif
 
+#if defined(__GNUC__) &&  ((__GNUC__ * 100 + __GNUC_MINOR__) >= 205)
+#define _X_UNUSED  __attribute__((__unused__))
+#else
+#define _X_UNUSED  /* */
+#endif
+
 /* C99 keyword "inline" or equivalent extensions in pre-C99 compilers */
 #if defined(inline) /* assume autoconf set it correctly */ || \
(defined(__STDC_VERSION__) && (__STDC_VERSION__ - 0 >= 199901L)) /* C99 */ 
|| \
-- 
1.7.4.1


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH 2/2] Add _X_NONNULL macro to annotate when a function expects arguments to be non-null

2011-04-27 Thread Jeremy Huddleston
This will allow with compiler optimization and better static analysis.

Signed-off-by: Jeremy Huddleston 
---

Alan, what SunCC magic needs to go here?

 Xfuncproto.h.in |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/Xfuncproto.h.in b/Xfuncproto.h.in
index 88d91c0..39ca3b2 100644
--- a/Xfuncproto.h.in
+++ b/Xfuncproto.h.in
@@ -122,6 +122,12 @@ in this Software without prior written authorization from 
The Open Group.
 # define _X_ATTRIBUTE_PRINTF(x,y)
 #endif
 
+#if defined(__GNUC__) &&  ((__GNUC__ * 100 + __GNUC_MINOR__) >= 303)
+#define _X_NONNULL(args...)  __attribute__((nonnull(args)))
+#else
+#define _X_NONNULL(...)  /* */
+#endif
+
 #if defined(__GNUC__) &&  ((__GNUC__ * 100 + __GNUC_MINOR__) >= 205)
 #define _X_UNUSED  __attribute__((__unused__))
 #else
-- 
1.7.4.1


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 1/6] make byte swapping macros less opaque to the optimizer

2011-04-27 Thread Jeremy Huddleston
For 1,2,4,5:
Reviewed-by: Jeremy Huddleston 
Tested-by: Jeremy Huddleston 

For 3,6:
Reviewed-by: Jeremy Huddleston 


On Apr 27, 2011, at 16:33, Matt Turner wrote:

> Text size reduction of ~33k (1.8%):
>   text   data bss dec hex filename
> 1879739 72212   55328 2007279  1ea0ef hw/xfree86/Xorg.before
> 1845291 72212   55328 1972831  1e1a5f hw/xfree86/Xorg
> 
> On x86[-64], the number of bswap instructions goes up from 5 to 1006.
> 
> By defining lswapl as gcc's builtin bswap32, the number of bswap
> instructions used goes up to 1025, but the text and binary sizes stay
> the same. This tells me that either gcc is
>   - dumb, and unable to optimize away __builtin_bswap32 when it should
>   - smart, and able to optimize away the open-coded bswap
> 
> Either way, it doesn't matter much.
> 
> Signed-off-by: Matt Turner 
> ---
> 
> The follow-up patch to this which removes the temporary variables used
> by the swap macros is too big for the mailing list, so please review it
> here:
> http://cgit.freedesktop.org/~mattst88/xserver/commit/?id=11ff6813cf2f2a04243f4eecb60078d77fc973ec
> 
> I have compile tested ephyr and dmx, but I wasn't able to compile test
> XQuartz, so please give it a test.
> 
> include/misc.h |   31 ---
> 1 files changed, 8 insertions(+), 23 deletions(-)
> 
> diff --git a/include/misc.h b/include/misc.h
> index 803f5ba..b7a3fd2 100644
> --- a/include/misc.h
> +++ b/include/misc.h
> @@ -240,32 +240,17 @@ xstrtokenize(const char *str, const char* separators);
> #define SwapRestL(stuff) \
> SwapLongs((CARD32 *)(stuff + 1), LengthRestL(stuff))
> 
> -/* byte swap a 32-bit value */
> -#define swapl(x, n) { \
> -  n = ((char *) (x))[0];\
> -  ((char *) (x))[0] = ((char *) (x))[3];\
> -  ((char *) (x))[3] = n;\
> -  n = ((char *) (x))[1];\
> -  ((char *) (x))[1] = ((char *) (x))[2];\
> -  ((char *) (x))[2] = n; }
> -
> -/* byte swap a short */
> -#define swaps(x, n) { \
> -  n = ((char *) (x))[0];\
> -  ((char *) (x))[0] = ((char *) (x))[1];\
> -  ((char *) (x))[1] = n; }
> -
> /* copy 32-bit value from src to dst byteswapping on the way */
> -#define cpswapl(src, dst) { \
> - ((char *)&(dst))[0] = ((char *) &(src))[3];\
> - ((char *)&(dst))[1] = ((char *) &(src))[2];\
> - ((char *)&(dst))[2] = ((char *) &(src))[1];\
> - ((char *)&(dst))[3] = ((char *) &(src))[0]; }
> +#define cpswapl(src, dst) (dst) = lswapl((src))
> 
> /* copy short from src to dst byteswapping on the way */
> -#define cpswaps(src, dst) { \
> -  ((char *) &(dst))[0] = ((char *) &(src))[1];\
> -  ((char *) &(dst))[1] = ((char *) &(src))[0]; }
> +#define cpswaps(src, dst) (dst) = lswaps((src))
> +
> +/* byte swap a 32-bit value */
> +#define swapl(x, n) (*(x)) = lswapl(*(x))
> +
> +/* byte swap a short */
> +#define swaps(x, n) (*(x)) = lswaps(*(x))
> 
> extern _X_EXPORT void SwapLongs(
> CARD32 *list,
> -- 
> 1.7.3.4
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PULL] XQuartz brownbag buildfix

2011-04-27 Thread Jeremy Huddleston
The following changes since commit f6d4e75ec55ac6812f9dead42ecdffb9614578c7:

  Merge remote-tracking branch 'jturney/master' (2011-04-27 12:08:51 -0700)

are available in the git repository at:

  git://people.freedesktop.org/~jeremyhu/xserver master

Jeremy Huddleston (1):
  XQuartz: BuildFix to build correctly with XPLUGIN_VERSION < 4

 hw/xquartz/applewmExt.h |   18 ++
 1 files changed, 6 insertions(+), 12 deletions(-)

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH font-util 1/2] bdftruncate: Properly support -w and +w

2011-04-28 Thread Jeremy Huddleston

Regression introduced by fb486bb1a5038912d064291b12c7aef5da3d8b63

Found-by: clang static analyzer
Signed-off-by: Jeremy Huddleston 
---
 bdftruncate.c |5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/bdftruncate.c b/bdftruncate.c
index a527305..2bd04f8 100644
--- a/bdftruncate.c
+++ b/bdftruncate.c
@@ -46,7 +46,6 @@ static void usage(void);
 
 static int opt_minus_w;
 static int opt_plus_w;
-static int removewide;
 static unsigned long threshold;
 
 static int
@@ -70,7 +69,7 @@ parse_threshold(const char *str)
 }
 
 static void
-process_line(const char *line)
+process_line(const char *line, int removewide)
 {
if (strncmp(line, "ENCODING", 8) == 0) {
unsigned long enc;
@@ -178,7 +177,7 @@ main(int argc, char **argv)
break;
}
}
-   process_line(line);
+   process_line(line, removewide);
}
 
return EXIT_SUCCESS;
-- 
1.7.4.1


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH font-util 2/2] ucs2any: Dead code removal

2011-04-28 Thread Jeremy Huddleston

Found-by: clang static analyzer
Signed-off-by: Jeremy Huddleston 
---
 ucs2any.c |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/ucs2any.c b/ucs2any.c
index a6e8fb6..cc60544 100644
--- a/ucs2any.c
+++ b/ucs2any.c
@@ -737,7 +737,6 @@ main(int argc, char *argv[])
if (p[0] == '0' && (p[1] == 'x' || p[1] == 'X')) {
ucs = strtol(p+2, &endp, 16);
if (*endp == '\0') goto bad;
-   p = endp;
} else
goto bad;
 
-- 
1.7.4.1


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH iceauth] Error out and avoid a call to malloc(0) if given a bad hex string

2011-04-28 Thread Jeremy Huddleston

Found-by: clang static analyzer
Signed-off-by: Jeremy Huddleston 
---
 process.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/process.c b/process.c
index f51e643..56b7aaf 100644
--- a/process.c
+++ b/process.c
@@ -401,8 +401,8 @@ static int cvthexkey (  /* turn hex key string into 
octets */
len++;
 }
 
-/* if odd then there was an error */
-if ((len & 1) == 1) return -1;
+/* if 0 or odd, then there was an error */
+if (len == 0 || (len & 1) == 1) return -1;
 
 
 /* now we know that the input is good */
-- 
1.7.4.1


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


clang static analysis updated with tinderbox

2011-04-28 Thread Jeremy Huddleston
I'm in the process of updating my tinderboxes to run clang static analysis and 
upload the results.  Currently only yuffie (XQuartz) has data available, and 
I'll be updating my linux tinerboxes in the next few days once yuffie is going 
smoothly.  Hopefully cjb can integrate this a bit nicer into the 
tinderbox.x.org site, but for now, you can see it on my p.fd.o webspace:

http://people.freedesktop.org/~jeremyhu/analyzer/

I've started going through the list (some patches already sent to xorg-devel 
for font-util and iceauth), but help is always welcome.  There are some tests 
where"we know better".  For those, hopefully the _X_UNUSED and _X_NONNULL 
macros will help tag things appropriately.  For the other cases (rand() 
security concerns), we can use ifdef-foo to silence the analyzer:

#ifndef __clang_analyzer__
// Code not to be analyzed
#endif

The changes to jhbuild are minor (comments welcome before I send it off to 
gnome bugzilla).

Thanks,
Jeremy




>From 8784873bb86f92cab7d0341892f5db4343eb68a0 Mon Sep 17 00:00:00 2001
From: Jeremy Huddleston 
Date: Thu, 28 Apr 2011 00:55:13 -0700
Subject: [PATCH] Support running scan-build (Clang Static Analyzer) with
 autotools projects

http://clang-analyzer.llvm.org

Signed-off-by: Jeremy Huddleston 
---
 jhbuild/config.py |3 ++-
 jhbuild/defaults.jhbuildrc|6 ++
 jhbuild/modtypes/autotools.py |   12 +---
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/jhbuild/config.py b/jhbuild/config.py
index f13e303..3e20436 100644
--- a/jhbuild/config.py
+++ b/jhbuild/config.py
@@ -58,7 +58,8 @@ _known_keys = [ 'moduleset', 'modules', 'skip', 'tags', 
'prefix',
 'jhbuildbot_mastercfg', 'use_local_modulesets',
 'ignore_suggests', 'modulesets_dir', 'mirror_policy',
 'module_mirror_policy', 'dvcs_mirror_dir', 'build_targets',
-'cmakeargs', 'module_cmakeargs' ]
+'cmakeargs', 'module_cmakeargs',
+'scan_build', 'module_scan_build', 'scan_buildargs', 
'scan_build_outputdir' ]
 
 env_prepends = {}
 def prependpath(envvar, path):
diff --git a/jhbuild/defaults.jhbuildrc b/jhbuild/defaults.jhbuildrc
index 7abe832..f9c34b7 100644
--- a/jhbuild/defaults.jhbuildrc
+++ b/jhbuild/defaults.jhbuildrc
@@ -89,6 +89,12 @@ interact  = True   # whether to interact with the user.
 quiet_mode= False  # whether to display running commands output
 progress_bar  = True   # whether to display a progress bar when running in 
quiet mode
 
+# Run clang static analyzer (scan-build), scan_build_outputdir has 
subdirectories for each module id
+scan_build= False
+module_scan_build = {}
+scan_buildargs = '-v'
+scan_build_outputdir = '/tmp/jhbuild_scan_build'
+
 # checkout modes. For VCS directories, it specifies how the checkout
 # is done. We can also specify checkout modes for specific modules
 checkout_mode = 'update'
diff --git a/jhbuild/modtypes/autotools.py b/jhbuild/modtypes/autotools.py
index 215df91..ff7a068 100644
--- a/jhbuild/modtypes/autotools.py
+++ b/jhbuild/modtypes/autotools.py
@@ -121,13 +121,14 @@ class AutogenModule(Package, DownloadableModule):
 if self.autogen_template:
 template = self.autogen_template
 else:
-template = ("%(srcdir)s/%(autogen-sh)s --prefix %(prefix)s"
+template = ("%(scan_build)s%(srcdir)s/%(autogen-sh)s --prefix 
%(prefix)s"
 " --libdir %(libdir)s %(autogenargs)s ")
 
 autogenargs = self.autogenargs + ' ' + 
self.config.module_autogenargs.get(
 self.name, self.config.autogenargs)
 
-vars = {'prefix': buildscript.config.prefix,
+vars = {'scan_build': self.scan_build_template(buildscript),
+'prefix': buildscript.config.prefix,
 'autogen-sh': self.autogen_sh,
 'autogenargs': autogenargs}
 
@@ -210,13 +211,18 @@ class AutogenModule(Package, DownloadableModule):
 buildscript.set_action(_('Building'), self)
 makeargs = self.makeargs + ' ' + self.config.module_makeargs.get(
 self.name, self.config.makeargs)
-cmd = '%s %s' % (os.environ.get('MAKE', 'make'), makeargs)
+cmd = '%s%s %s' % (self.scan_build_template(buildscript), 
os.environ.get('MAKE', 'make'), makeargs)
 buildscript.execute(cmd, cwd = self.get_builddir(buildscript),
 extra_env = self.extra_env)
 do_build.depends = [PHASE_CONFIGURE]
 do_build.error_phases = [

[PATCH font-util 1/2 v2] bdftruncate: Properly support -w and +w

2011-04-28 Thread Jeremy Huddleston

Regression introduced by fb486bb1a5038912d064291b12c7aef5da3d8b63

Found-by: clang static analyzer
Signed-off-by: Jeremy Huddleston 
---
 bdftruncate.c |   21 ++---
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/bdftruncate.c b/bdftruncate.c
index a527305..e2b5d87 100644
--- a/bdftruncate.c
+++ b/bdftruncate.c
@@ -43,14 +43,10 @@
 
 static int iswide(unsigned int);
 static void usage(void);
-
-static int opt_minus_w;
-static int opt_plus_w;
-static int removewide;
-static unsigned long threshold;
+static int parse_threshold(const char *str, unsigned long *threshold);
 
 static int
-parse_threshold(const char *str)
+parse_threshold(const char *str, unsigned long *threshold)
 {
int base;
char *end_ptr;
@@ -63,14 +59,14 @@ parse_threshold(const char *str)
base = 10;
 
errno = 0;
-   threshold = strtoul(str, &end_ptr, base);
-   if (errno != 0 || threshold == 0)
+   *threshold = strtoul(str, &end_ptr, base);
+   if (errno != 0 || *threshold == 0)
return 1;
return 0;
 }
 
 static void
-process_line(const char *line)
+process_line(const char *line, int removewide, unsigned long threshold)
 {
if (strncmp(line, "ENCODING", 8) == 0) {
unsigned long enc;
@@ -120,6 +116,9 @@ int
 main(int argc, char **argv)
 {
int removewide;
+   unsigned long threshold;
+   int opt_minus_w;
+   int opt_plus_w;
char *line, *input_ptr;
size_t line_len, rest_len;
 
@@ -139,7 +138,7 @@ main(int argc, char **argv)
 
if (argc != 1 || (opt_plus_w && opt_minus_w))
usage();
-   if (parse_threshold(*argv)) {
+   if (parse_threshold(*argv, &threshold)) {
fprintf(stderr, "Illegal threshold %s\n", *argv);
usage();
}
@@ -178,7 +177,7 @@ main(int argc, char **argv)
break;
}
}
-   process_line(line);
+   process_line(line, removewide, threshold);
}
 
return EXIT_SUCCESS;
-- 
1.7.4.1


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: doxygen help (xserver won't make dist due to missing hw/dmx/doc/html/main.html)

2011-04-28 Thread Jeremy Huddleston
Yep, +1 for nuking generated doc files from EXTRA_DIST

I recall Gaetan sent a patch to xorg-devel to do that (which I believe I 
reviewed).  It doesn't look like it has landed yet though.

On Apr 28, 2011, at 09:12, Jon TURNEY wrote:

> 
> 
> On 29/03/2011 14:13, Gaetan Nadon wrote:
>> On Mon, 2011-03-28 at 23:29 -0700, Jeremy Huddleston wrote:
>> 
>>> As a followup, it seems that my 'make dist' in recent 1.9.x releases
>>> was actually not rebuilding hw/dmx/doc/html but using an existing
>>> version built back in October.  I'm only noticing this now because I
>>> did a new checkout for 1.10 and thus don't have the old files.
>> 
>> make distclean will remove the generated html files.
>> 
>>> 
>>> My guess is that older doxygen build main.html, and newer ones build
>>> index.html, so I'm just updating EXTRA_DIST with the updated list of
>>> generated files and will use that... expect 1.10.1 RC1 tomorrow.
>>> Sorry for the delay.
> 
> Having just got burnt by this as well, this seems to be a change in doxygen
> 1.7.3.
> 
> The changelog [1] line "Redesigned the treeview feature. Instead of using
> frames, the navigation tree is now loaded on each page dynamically." seems to
> mean "replaced index.html which loads tree.html and main.html in frames with
> one which uses javascript"
> 
>> If what you suspect is correct, someone else will have the same problem
>> with the new list in EXTRA_DIST. There is no guarantee that the list of
>> generated files will remain the same over time. I have been advocating
>> not to include these generated files in the tarball. I have not seen
>> evidence that any distro is missing doxygen to build these files. The
>> time and effort spent to maintain this is disproportionate with the
>> benefits (I suspect none) it brings.
> 
> Anyhow, Gaetan is absolutely correct, listing the files we expect doxygen to
> generate in EXTRA_DIST in hw/dmx/doc/Makefile.am isn't robust against changes
> like this
> 
> [1] http://www.stack.nl/~dimitri/doxygen/changelog.html
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH libX11] docbook: Don't use --nonet in XSLTPROC_FLAGS

2011-04-28 Thread Jeremy Huddleston

We are explicitly pulling in resources from the net, so this would always fail.

%.html.db: %.xml  $(chapters)
$(AM_V_GEN)$(XSLTPROC) $(XSLTPROC_FLAGS) \
http://docbook.sourceforge.net/release/xsl/current/xhtml/docbook.xsl $<

%.fo.db: %.xml $(chapters)
$(AM_V_GEN)$(XSLTPROC) $(XSLTPROC_FLAGS) \
http://docbook.sourceforge.net/release/xsl/current/fo/docbook.xsl $<

Signed-off-by: Jeremy Huddleston 
---
 docbook.am |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/docbook.am b/docbook.am
index 6937083..0fd6af1 100644
--- a/docbook.am
+++ b/docbook.am
@@ -80,7 +80,7 @@ XSLTPROC_FLAGS =  \
--path "$(XORG_SGML_PATH)/X11"  \
--stringparam targets.filename "$@" \
--stringparam collect.xref.targets "only"   \
-   --nonet --xinclude
+   --xinclude
 
 %.html.db: %.xml  $(chapters)
$(AM_V_GEN)$(XSLTPROC) $(XSLTPROC_FLAGS) \
-- 
1.7.4.4


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libX11] docbook: Don't use --nonet in XSLTPROC_FLAGS

2011-04-28 Thread Jeremy Huddleston

On Apr 28, 2011, at 5:25 PM, Dan Nicholson wrote:

> This isn't true. If your xml catalog is setup correctly, the url will
> be converted to a local file. In /etc/xml/catalog on my F13 box:
> 
>   uriStartString="http://docbook.sourceforge.net/release/xsl/current";
> rewritePrefix="file:///usr/share/sgml/docbook/xsl-stylesheets-1.75.2"/>

Ok, then it's a config issue on my end...

After getting past that, I hit errors where xmlto is trying to parse the HTTP 
transcript rather than the returned value (it's barfing on the HTTP 200 OK).

> This is no different than having an http:// URI at the top of an xml
> document. Others may feel differently, but I think it's much preferred
> to not start pulling bits off the network during a build. If you run
> xmlto (rather than xsltproc directly), it will always pass --nonet to
> xsltproc.

Alright, then I need to figure out why my system isn't rewriting those 
correctly... sigh...

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libX11] docbook: Don't use --nonet in XSLTPROC_FLAGS

2011-04-28 Thread Jeremy Huddleston
> This is what I observed as well. The xsltproc flags were derived from
> the xsltproc command
> issued by xmlto which is just a wrapper srcipt.

It looks like a problem with the catalog installed with docbook-xsl-1.76.1 ... 
ugg... I hate generating docs.



  
  http://docbook.sourceforge.net/release/xsl/current/"; 
rewritePrefix="./"/>
  http://docbook.sourceforge.net/release/xsl/current/"; 
rewritePrefix="./"/>
  http://docbook.sourceforge.net/release/xsl/1.76.1/"; 
rewritePrefix="./"/>
  http://docbook.sourceforge.net/release/xsl/1.76.1/"; 
rewritePrefix="./"/>



___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: doxygen help (xserver won't make dist due to missing hw/dmx/doc/html/main.html)

2011-04-28 Thread Jeremy Huddleston

On Apr 28, 2011, at 5:52 PM, Gaetan Nadon wrote:

> On Thu, 2011-04-28 at 09:45 -0700, Jeremy Huddleston wrote:
> 
>> I recall Gaetan sent a patch to xorg-devel to do that (which I believe
>> I reviewed).  It doesn't look like it has landed yet though.
> 
> I could not find any review tag, sorry if I missed it. I reposted
> anyway.

I think what happened was I reviewed all but one (which was large and held for 
moderation) and I never got around to reviewing the entire series.  It looks 
like that happened again, but I found #4 in the archives.

Reviewed-by Jeremy Huddleston 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PULL] XQuartz: rwlock our window hash instead of mutex

2011-04-29 Thread Jeremy Huddleston
Also includes one more brown-bag fix for XPLUGIN_VERSION < 4 ... this time I 
made absolutely sure (I think...) that I was using an old version of the library

The following changes since commit 5cb31cd0cbf83fff5f17a475e7b0e45246b19bf3:

  Merge remote-tracking branch 'jturney/remove-opengl-spec-download' 
(2011-04-29 09:59:49 -0700)

are available in the git repository at:

  git://people.freedesktop.org/~jeremyhu/xserver master

Jeremy Huddleston (2):
  XQuartz: Use a rwlock instead of a mutex to protect window_hash in the 
pthread case
  XQuartz: Fix incorrect typedefs with XPLUGIN_VERSION < 4

 hw/xquartz/applewmExt.h   |6 +++---
 hw/xquartz/xpr/xprFrame.c |   20 ++--
 2 files changed, 13 insertions(+), 13 deletions(-)

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: clang static analysis updated with tinderbox

2011-04-29 Thread Jeremy Huddleston

On Apr 29, 2011, at 1:35 AM, Dirk Wallenstein wrote:

>> --- a/jhbuild/modtypes/autotools.py
>> +++ b/jhbuild/modtypes/autotools.py
>> @@ -121,13 +121,14 @@ class AutogenModule(Package, DownloadableModule):
>> if self.autogen_template:
>> template = self.autogen_template
> If this is configured, %(scan_build) won't be added. Would it make sense
> to add it independently, after this branching?

I would expect that %(scan_build) would be provided in self.autogen_template in 
the modules xml file if it was desired.  Is that not custom?  I'm not really 
sure what should take precedence here.

>> +def scan_build_template(self, buildscript):
>> +if self.name in buildscript.config.module_scan_build or 
>> buildscript.config.scan_build:
> I would say the module specific setting should override the global one,
> something like:
> 
>if buildscript.config.module_scan_build.get(self.name, 
> buildscript.config.scan_build):

Great, thanks.


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: clang static analysis updated with tinderbox

2011-04-30 Thread Jeremy Huddleston

On Apr 30, 2011, at 00:05, Dirk Wallenstein wrote:

> (Ups, new-mail-notification must have failed when visiting my old OS)
> I don't know clang but it looks like the command can be prefixed to any
> autogen.sh command, so that there is no need to edit existing
> modulesets. It seems that it would be possible to simply prefix it to
> the command without going through the template.
> 
>cmd = self.scan_build_template(buildscript) + cmd

Thanks for the suggestions.  I've incorporated feedback from here and sent the 
patch to gnome's bugzilla for further comments and eventual integration.

https://bugzilla.gnome.org/show_bug.cgi?id=648990

--Jeremy

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH:macros v2] Make XORG_STRICT_OPTION always set STRICT_CFLAGS

2011-05-02 Thread Jeremy Huddleston
Hey Alan,

I think you missed my comment about adding -Werror=attributes for the $GCC 
case.  Could you please add that as well (gcc doesn't default to 
-Werror=attributes with -Werror).

On May 1, 2011, at 10:36, Alan Coopersmith wrote:

> Still only adds it to CWARNFLAGS if --enable-strict-compilation is
> passed, but sets the variable with the right flags for the compiler
> so it's available for other checks in configure scripts.
> 
> Signed-off-by: Alan Coopersmith 
> ---
> 
> By popular request, adds AC_SUBST([STRICT_CFLAGS])
> Also updates comments to document that 1.14 will be the first version to
> export this, for future reference by people who want to use this.
> 
> xorg-macros.m4.in |   25 -
> 1 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/xorg-macros.m4.in b/xorg-macros.m4.in
> index bdf734e..ccbe931 100644
> --- a/xorg-macros.m4.in
> +++ b/xorg-macros.m4.in
> @@ -1337,7 +1337,13 @@ AC_SUBST(CWARNFLAGS)
> # ---
> # Minimum version: 1.3.0
> #
> -# Add configure option to enable strict compilation
> +# Add configure option to enable strict compilation flags, such as treating
> +# warnings as fatal errors.
> +# If --enable-strict-compilation is passed to configure, adds strict flags to
> +# $CWARNFLAGS.
> +#
> +# Starting in 1.14.0 also exports $STRICT_CFLAGS for use in other tests or
> +# when strict compilation is unconditionally desired.
> AC_DEFUN([XORG_STRICT_OPTION], [
> # If the module's configure.ac calls AC_PROG_CC later on, CC gets set to C89
> AC_REQUIRE([AC_PROG_CC_C99])
> @@ -1348,16 +1354,17 @@ AC_ARG_ENABLE(strict-compilation,
> AS_HELP_STRING([--enable-strict-compilation],
> [Enable all warnings from compiler and make them 
> errors (default: disabled)]),
> [STRICT_COMPILE=$enableval], [STRICT_COMPILE=no])
> +if test "x$GCC" = xyes ; then
> +STRICT_CFLAGS="-pedantic -Werror"
> +elif test "x$SUNCC" = "xyes"; then
> +STRICT_CFLAGS="-errwarn"
> +elif test "x$INTELCC" = "xyes"; then
> +STRICT_CFLAGS="-Werror"
> +fi
> if test "x$STRICT_COMPILE" = "xyes"; then
> - if test "x$GCC" = xyes ; then
> - STRICT_CFLAGS="-pedantic -Werror"
> - elif test "x$SUNCC" = "xyes"; then
> - STRICT_CFLAGS="-errwarn"
> - elif test "x$INTELCC" = "xyes"; then
> - STRICT_CFLAGS="-Werror"
> - fi
> +CWARNFLAGS="$CWARNFLAGS $STRICT_CFLAGS"
> fi
> -CWARNFLAGS="$CWARNFLAGS $STRICT_CFLAGS"
> +AC_SUBST([STRICT_CFLAGS])
> AC_SUBST([CWARNFLAGS])
> ]) # XORG_STRICT_OPTION
> 
> -- 
> 1.7.3.2
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH:macros v2] Make XORG_STRICT_OPTION always set STRICT_CFLAGS

2011-05-02 Thread Jeremy Huddleston
Sure, a second logical commit sounds good.

Reviewed-by: Jeremy Huddleston 

On May 2, 2011, at 15:36, Alan Coopersmith wrote:

> On 05/ 2/11 12:19 AM, Jeremy Huddleston wrote:
>> Hey Alan,
>> 
>> I think you missed my comment about adding -Werror=attributes for the $GCC 
>> case.  Could you please add that as well (gcc doesn't default to 
>> -Werror=attributes with -Werror).
> 
> I thought that was only needed in the xorg-tls.m4, but I guess it makes sense
> as part of the general case.   That seems like a separate logical commit 
> though,
> not part of exporting the STRICT_CFLAGS.
> 
> -- 
>   -Alan Coopersmith-alan.coopersm...@oracle.com
>Oracle Solaris Platform Engineering: X Window System
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


1.10.2 RC1 scheduled for Friday

2011-05-02 Thread Jeremy Huddleston
Send nominations and pull requests if you've been sitting on them.

Thanks,
Jeremy

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH:macros] XORG_STRICT_OPTION: add -Werror=attributes to STRICT_CFLAGS for gcc

2011-05-02 Thread Jeremy Huddleston
Thanks.

Reviewed-by: Jeremy Huddleston 

On May 2, 2011, at 7:13 PM, Alan Coopersmith wrote:

> Flags unknown attributes as errors, if -Werror=attributes is supported.
> (The -Werror=* option was first spotted in gcc 4.2.0 manuals.)
> 
> Signed-off-by: Alan Coopersmith 
> ---
> xorg-macros.m4.in |9 +
> 1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/xorg-macros.m4.in b/xorg-macros.m4.in
> index ccbe931..f89efb6 100644
> --- a/xorg-macros.m4.in
> +++ b/xorg-macros.m4.in
> @@ -1356,6 +1356,15 @@ AC_ARG_ENABLE(strict-compilation,
> [STRICT_COMPILE=$enableval], [STRICT_COMPILE=no])
> if test "x$GCC" = xyes ; then
> STRICT_CFLAGS="-pedantic -Werror"
> +# Add -Werror=attributes if supported (gcc 4.2 & later)
> +AC_MSG_CHECKING([if $CC supports -Werror=attributes])
> +save_CFLAGS="$CFLAGS"
> +CFLAGS="$CFLAGS $STRICT_CFLAGS -Werror=attributes"
> +AC_COMPILE_IFELSE([AC_LANG_SOURCE([return 0;])],
> +   [STRICT_CFLAGS="$STRICT_CFLAGS -Werror=attributes"
> +AC_MSG_RESULT([yes])],
> +   [AC_MSG_RESULT([no])])
> +CFLAGS="$save_CFLAGS"
> elif test "x$SUNCC" = "xyes"; then
> STRICT_CFLAGS="-errwarn"
> elif test "x$INTELCC" = "xyes"; then
> -- 
> 1.7.3.2
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libXi] Allocate enough memory for raw events + extra data.

2011-05-02 Thread Jeremy Huddleston
Woot.  That takes care of:

http://people.freedesktop.org/~jeremyhu/analyzer/yuffie/20110429-1617/libXi/report-kzyPla.html#EndPath

It looks like there are others you might want to do at the same time:
http://people.freedesktop.org/~jeremyhu/analyzer/yuffie/20110429-1617/libXi/

On May 2, 2011, at 8:50 PM, Peter Hutterer wrote:

> Necessary space was calculated, but not actually used to allocate memory. As
> a result, valuator data would overwrite the allocated memory.
> 
> ==4166== Invalid write of size 1
> ==4166==at 0x4C29F04: memcpy (mc_replace_strmem.c:497)
> ==4166==by 0x8F39180: ??? (in /usr/lib/libXi.so.6.1.0)
> ==4166==by 0x7433D48: _XCopyEventCookie (in /usr/lib/libX11.so.6.3.0)
> ==4166==by 0x7425166: XPeekEvent (in /usr/lib/libX11.so.6.3.0)
> ==4166==by 0x49C3E3: process_key (x11_be.c:1065)
> ==4166==by 0x49EA5C: event_key_release (x11_be.c:2201)
> ==4166==by 0x49DD6E: x11_be_process_events (x11_be.c:1892)
> ==4166==by 0x4A38F4: x11_be_main_loop (x11_be.c:4353)
> ==4166==by 0x4A39E1: x11_be_thread_main (x11_be.c:4385)
> ==4166==by 0x87549C9: start_thread (pthread_create.c:300)
> ==4166==by 0x8A516FC: clone (clone.S:112)
> ==4166==  Address 0x168afe80 is 0 bytes after a block of size 96 alloc'd
> ==4166==at 0x4C284A8: malloc (vg_replace_malloc.c:236)
> ==4166==by 0x8F390BD: ??? (in /usr/lib/libXi.so.6.1.0)
> ==4166==by 0x7433D48: _XCopyEventCookie (in /usr/lib/libX11.so.6.3.0)
> ==4166==by 0x7425166: XPeekEvent (in /usr/lib/libX11.so.6.3.0)
> ==4166==by 0x49C3E3: process_key (x11_be.c:1065)
> ==4166==by 0x49EA5C: event_key_release (x11_be.c:2201)
> ==4166==by 0x49DD6E: x11_be_process_events (x11_be.c:1892)
> ==4166==by 0x4A38F4: x11_be_main_loop (x11_be.c:4353)
> ==4166==by 0x4A39E1: x11_be_thread_main (x11_be.c:4385)
> ==4166==by 0x87549C9: start_thread (pthread_create.c:300)
> 
> Reported-by: Roger Cruz 
> Signed-off-by: Peter Hutterer 
> ---
> Sorry, was away for a number of days and only got to this now. You're right,
> it's a bug, copy/paste I guess.
> 
> src/XExtInt.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/src/XExtInt.c b/src/XExtInt.c
> index d1451cc..134ccc6 100644
> --- a/src/XExtInt.c
> +++ b/src/XExtInt.c
> @@ -1259,7 +1259,7 @@ copyRawEvent(XGenericEventCookie *cookie_in,
> len = sizeof(XIRawEvent) + in->valuators.mask_len;
> len += bits * sizeof(double) * 2;
> 
> -ptr = cookie_out->data = malloc(sizeof(XIRawEvent));
> +ptr = cookie_out->data = malloc(len);
> if (!ptr)
> return False;
> 
> -- 
> 1.7.4.4
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libXi] Allocate enough memory for raw events + extra data.

2011-05-02 Thread Jeremy Huddleston
Oh yeah, and:

Reviewed-by: Jeremy Huddleston 

On May 2, 2011, at 8:50 PM, Peter Hutterer wrote:

> Necessary space was calculated, but not actually used to allocate memory. As
> a result, valuator data would overwrite the allocated memory.
> 
> ==4166== Invalid write of size 1
> ==4166==at 0x4C29F04: memcpy (mc_replace_strmem.c:497)
> ==4166==by 0x8F39180: ??? (in /usr/lib/libXi.so.6.1.0)
> ==4166==by 0x7433D48: _XCopyEventCookie (in /usr/lib/libX11.so.6.3.0)
> ==4166==by 0x7425166: XPeekEvent (in /usr/lib/libX11.so.6.3.0)
> ==4166==by 0x49C3E3: process_key (x11_be.c:1065)
> ==4166==by 0x49EA5C: event_key_release (x11_be.c:2201)
> ==4166==by 0x49DD6E: x11_be_process_events (x11_be.c:1892)
> ==4166==by 0x4A38F4: x11_be_main_loop (x11_be.c:4353)
> ==4166==by 0x4A39E1: x11_be_thread_main (x11_be.c:4385)
> ==4166==by 0x87549C9: start_thread (pthread_create.c:300)
> ==4166==by 0x8A516FC: clone (clone.S:112)
> ==4166==  Address 0x168afe80 is 0 bytes after a block of size 96 alloc'd
> ==4166==at 0x4C284A8: malloc (vg_replace_malloc.c:236)
> ==4166==by 0x8F390BD: ??? (in /usr/lib/libXi.so.6.1.0)
> ==4166==by 0x7433D48: _XCopyEventCookie (in /usr/lib/libX11.so.6.3.0)
> ==4166==by 0x7425166: XPeekEvent (in /usr/lib/libX11.so.6.3.0)
> ==4166==by 0x49C3E3: process_key (x11_be.c:1065)
> ==4166==by 0x49EA5C: event_key_release (x11_be.c:2201)
> ==4166==by 0x49DD6E: x11_be_process_events (x11_be.c:1892)
> ==4166==by 0x4A38F4: x11_be_main_loop (x11_be.c:4353)
> ==4166==by 0x4A39E1: x11_be_thread_main (x11_be.c:4385)
> ==4166==by 0x87549C9: start_thread (pthread_create.c:300)
> 
> Reported-by: Roger Cruz 
> Signed-off-by: Peter Hutterer 
> ---
> Sorry, was away for a number of days and only got to this now. You're right,
> it's a bug, copy/paste I guess.
> 
> src/XExtInt.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/src/XExtInt.c b/src/XExtInt.c
> index d1451cc..134ccc6 100644
> --- a/src/XExtInt.c
> +++ b/src/XExtInt.c
> @@ -1259,7 +1259,7 @@ copyRawEvent(XGenericEventCookie *cookie_in,
> len = sizeof(XIRawEvent) + in->valuators.mask_len;
> len += bits * sizeof(double) * 2;
> 
> -ptr = cookie_out->data = malloc(sizeof(XIRawEvent));
> +ptr = cookie_out->data = malloc(len);
> if (!ptr)
> return False;
> 
> -- 
> 1.7.4.4
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 1/2] Use XORG_COMPILER_BRAND from util-macros 1.14 to check for SUNCC

2011-05-02 Thread Jeremy Huddleston
Reviewed-by: Jeremy Huddleston 

On May 2, 2011, at 8:54 PM, Alan Coopersmith wrote:

> Signed-off-by: Alan Coopersmith 
> ---
> 
> To test with current util/macros git, bump the version in its configure.ac
> to 1.14.0 and install first.
> 
> configure.ac |8 +++-
> 1 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 61caaed..bd1bb0b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -32,10 +32,10 @@ AC_CONFIG_SRCDIR([Makefile.am])
> AM_INIT_AUTOMAKE([foreign dist-bzip2])
> AM_MAINTAINER_MODE
> 
> -# Require xorg-macros minimum of 1.13 for XORG_ENABLE_UNIT_TESTS
> +# Require xorg-macros minimum of 1.14 for XORG_COMPILER_BRAND in 
> XORG_DEFAULT_OPTIONS
> m4_ifndef([XORG_MACROS_VERSION],
> -  [m4_fatal([must install xorg-macros 1.13 or later before running 
> autoconf/autogen])])
> -XORG_MACROS_VERSION(1.13)
> +  [m4_fatal([must install xorg-macros 1.14 or later before running 
> autoconf/autogen])])
> +XORG_MACROS_VERSION(1.14)
> XORG_DEFAULT_OPTIONS
> XORG_WITH_DOXYGEN(1.6.1)
> XORG_CHECK_SGML_DOCTOOLS(1.5)
> @@ -1522,7 +1522,6 @@ if test "x$XORG" = xyes; then
>   if test x$GCC = xyes; then
>   VISIBILITY_CFLAGS="-fvisibility=hidden"
>   else
> - AC_CHECK_DECL([__SUNPRO_C], [SUNCC="yes"], [SUNCC="no"])
>   if test x$SUNCC = xyes; then
>   VISIBILITY_CFLAGS="-xldscope=hidden"
>   else
> @@ -1631,7 +1630,6 @@ if test "x$XORG" = xyes; then
>   if test "${OS_MINOR}" -lt 8 ; then
>   AC_MSG_ERROR([This release no longer supports Solaris 
> versions older than Solaris 8.])
>   fi
> - AC_CHECK_DECL([__SUNPRO_C], [SUNCC="yes"], [SUNCC="no"])
>   if test "x$SUNCC" = "xyes"; then
>   solaris_asm_inline="yes"
>   fi
> -- 
> 1.7.3.2
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 2/2] Use XORG_STRICT_OPTION from util-macros 1.14 to set -Werror flags

2011-05-02 Thread Jeremy Huddleston
Reviewed-by: Jeremy Huddleston 

Thanks.

On May 2, 2011, at 8:54 PM, Alan Coopersmith wrote:

> Signed-off-by: Alan Coopersmith 
> ---
> 
> Assumes PATCH 1/2 has already established the baseline of macros 1.14 in
> configure.ac (though it mostly works fine without it, just allows unknown
> attribute warnings to appear on compilers that don't support them but don't
> make them fatal errors).
> 
> m4/xorg-tls.m4 |6 ++
> 1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/m4/xorg-tls.m4 b/m4/xorg-tls.m4
> index 5638504..237fdcd 100644
> --- a/m4/xorg-tls.m4
> +++ b/m4/xorg-tls.m4
> @@ -22,6 +22,7 @@ dnl
> dnl Authors: Jeremy Huddleston 
> 
> AC_DEFUN([XORG_TLS], [
> +AC_REQUIRE([XORG_STRICT_OPTION])
> AC_MSG_CHECKING(for thread local storage (TLS) support)
> AC_CACHE_VAL(ac_cv_tls, [
> ac_cv_tls=none
> @@ -36,10 +37,7 @@ AC_DEFUN([XORG_TLS], [
> AC_MSG_CHECKING(for tls_model attribute support)
> AC_CACHE_VAL(ac_cv_tls_model, [
> save_CFLAGS="$CFLAGS"
> -dnl -Werror causes clang's default -Wunknown-attributes to 
> become an error
> -dnl We can't use -Werror=unknown-attributes because gcc doesn't 
> understand it
> -dnl -Werror=attributes is for gcc, clang seems to ignore it
> -CFLAGS="$CFLAGS -Werror -Werror=attributes"
> +CFLAGS="$CFLAGS $STRICT_CFLAGS"
> AC_TRY_COMPILE([int $ac_cv_tls 
> __attribute__((tls_model("initial-exec"))) test;], [],
>ac_cv_tls_model=yes, ac_cv_tls_model=no)
> CFLAGS="$save_CFLAGS"
> -- 
> 1.7.3.2
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libXi] Remove a few unused assignments.

2011-05-02 Thread Jeremy Huddleston
Reviewed-by: Jeremy Huddleston 

On May 2, 2011, at 9:55 PM, Peter Hutterer wrote:

> Found by static analyzer.
> 
> Reported-by: Jeremy Huddleston 
> Signed-off-by: Peter Hutterer 
> ---
> src/XExtInt.c |3 ---
> 1 files changed, 0 insertions(+), 3 deletions(-)
> 
> diff --git a/src/XExtInt.c b/src/XExtInt.c
> index 134ccc6..55144c6 100644
> --- a/src/XExtInt.c
> +++ b/src/XExtInt.c
> @@ -760,7 +760,6 @@ XInputWireToEvent(
> return (DONT_ENQUEUE);
> else {
> *re = *save;
> -stev = (XDeviceStateNotifyEvent *) re;
> return (ENQUEUE_EVENT);
> }
> }
> @@ -788,7 +787,6 @@ XInputWireToEvent(
> return (DONT_ENQUEUE);
> else {
> *re = *save;
> -kstev = (XDeviceStateNotifyEvent *) re;
> return (ENQUEUE_EVENT);
> }
> }
> @@ -816,7 +814,6 @@ XInputWireToEvent(
> return (DONT_ENQUEUE);
> else {
> *re = *save;
> -bstev = (XDeviceStateNotifyEvent *) re;
> return (ENQUEUE_EVENT);
> }
> }
> -- 
> 1.7.4.4
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libXi] XIChangeHierarchy: Return Success early if no actual changes are requested.

2011-05-02 Thread Jeremy Huddleston
Reviewed-by: Jeremy Huddleston 

On May 2, 2011, at 9:58 PM, Peter Hutterer wrote:

> Do the same for negative num_changes.
> 
> Signed-off-by: Peter Hutterer 
> ---
> man/XIChangeHierarchy.txt |3 ++-
> src/XIHierarchy.c |3 +++
> 2 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/man/XIChangeHierarchy.txt b/man/XIChangeHierarchy.txt
> index ac667bc..205f40f 100644
> --- a/man/XIChangeHierarchy.txt
> +++ b/man/XIChangeHierarchy.txt
> @@ -30,7 +30,8 @@ DESCRIPTION
>XIChangeHierarchy modifies the device hierarchy by creating or
>removing master devices or changing the attachment of slave
>devices. If num_changes is non-zero, changes is an array of
> -   XIAnyHierarchyChangeInfo structures.
> +   XIAnyHierarchyChangeInfo structures. If num_changes is equal or less than
> +   zero, XIChangeHierarchy does nothing.
> 
>XIChangeHierarchy processes changes in order, effective
>immediately. If an error occurs, processing is aborted and the
> diff --git a/src/XIHierarchy.c b/src/XIHierarchy.c
> index d30ea29..09e6f93 100644
> --- a/src/XIHierarchy.c
> +++ b/src/XIHierarchy.c
> @@ -52,6 +52,9 @@ XIChangeHierarchy(Display* dpy,
> if (_XiCheckExtInit(dpy, XInput_2_0, info) == -1)
>   return (NoSuchExtension);
> 
> +if (num_changes <= 0)
> +return Success;
> +
> GetReq(XIChangeHierarchy, req);
> req->reqType = info->codes->major_opcode;
> req->ReqType = X_XIChangeHierarchy;
> -- 
> 1.7.4.4
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libXi] Allocate enough memory for raw events + extra data.

2011-05-02 Thread Jeremy Huddleston

On May 2, 2011, at 10:00 PM, Peter Hutterer wrote:

> On Mon, May 02, 2011 at 09:05:21PM -0700, Jeremy Huddleston wrote:
>> Woot.  That takes care of:
>> 
>> http://people.freedesktop.org/~jeremyhu/analyzer/yuffie/20110429-1617/libXi/report-kzyPla.html#EndPath
>> 
>> It looks like there are others you might want to do at the same time:
>> http://people.freedesktop.org/~jeremyhu/analyzer/yuffie/20110429-1617/libXi/
> 
> re "Idempotent operation"
> I'm staring hard at those but don't really see the issue.
> SetReqLen(req, foo, foo); is valid and for any foo that is non-zero
> shouldn't have the effect of "Assigned value is always the same as the
> existing value". SetReqLen essentially does req->length += foo.

Yeah... "within expansion of macro..." results are a bit bad with clang.  I 
just opened a bug about that.  Hopefully future analytics will be better.  For 
now, at worse it's a dead assignment, and we can come back to it when clang 
gets better at printing those results.

> 
> http://people.freedesktop.org/~jeremyhu/analyzer/yuffie/20110429-1617/libXi/report-yDDyYj.html#EndPath
> "undefined allocation of 0 bytes" would be a server bug afaict

I dunno... is there anywhere that we guarantee that Xmalloc(0) returns NULL?  
If so, then the macro should probably be ((size) > 0 ? malloc(size) : NULL) ... 
hopefully there is nothing actually using side-effects inside of Xmalloc(...) 
...

> http://people.freedesktop.org/~jeremyhu/analyzer/yuffie/20110429-1617/libXi/report-n6GgtE.html#EndPath
> don't want to change this one, it's there for symmetry.

Would you mind doing this to shut up the warning?  I know it's ugly, but there 
unfortunately isn't a better way to whitelist in the analyzer yet:

#ifndef __clang_analyzer__
...
#endif


> 
> Cheers,
>  Peter
> 
>> On May 2, 2011, at 8:50 PM, Peter Hutterer wrote:
>> 
>>> Necessary space was calculated, but not actually used to allocate memory. As
>>> a result, valuator data would overwrite the allocated memory.
>>> 
>>> ==4166== Invalid write of size 1
>>> ==4166==at 0x4C29F04: memcpy (mc_replace_strmem.c:497)
>>> ==4166==by 0x8F39180: ??? (in /usr/lib/libXi.so.6.1.0)
>>> ==4166==by 0x7433D48: _XCopyEventCookie (in /usr/lib/libX11.so.6.3.0)
>>> ==4166==by 0x7425166: XPeekEvent (in /usr/lib/libX11.so.6.3.0)
>>> ==4166==by 0x49C3E3: process_key (x11_be.c:1065)
>>> ==4166==by 0x49EA5C: event_key_release (x11_be.c:2201)
>>> ==4166==by 0x49DD6E: x11_be_process_events (x11_be.c:1892)
>>> ==4166==by 0x4A38F4: x11_be_main_loop (x11_be.c:4353)
>>> ==4166==by 0x4A39E1: x11_be_thread_main (x11_be.c:4385)
>>> ==4166==by 0x87549C9: start_thread (pthread_create.c:300)
>>> ==4166==by 0x8A516FC: clone (clone.S:112)
>>> ==4166==  Address 0x168afe80 is 0 bytes after a block of size 96 alloc'd
>>> ==4166==at 0x4C284A8: malloc (vg_replace_malloc.c:236)
>>> ==4166==by 0x8F390BD: ??? (in /usr/lib/libXi.so.6.1.0)
>>> ==4166==by 0x7433D48: _XCopyEventCookie (in /usr/lib/libX11.so.6.3.0)
>>> ==4166==by 0x7425166: XPeekEvent (in /usr/lib/libX11.so.6.3.0)
>>> ==4166==by 0x49C3E3: process_key (x11_be.c:1065)
>>> ==4166==by 0x49EA5C: event_key_release (x11_be.c:2201)
>>> ==4166==by 0x49DD6E: x11_be_process_events (x11_be.c:1892)
>>> ==4166==by 0x4A38F4: x11_be_main_loop (x11_be.c:4353)
>>> ==4166==by 0x4A39E1: x11_be_thread_main (x11_be.c:4385)
>>> ==4166==by 0x87549C9: start_thread (pthread_create.c:300)
>>> 
>>> Reported-by: Roger Cruz 
>>> Signed-off-by: Peter Hutterer 
>>> ---
>>> Sorry, was away for a number of days and only got to this now. You're right,
>>> it's a bug, copy/paste I guess.
>>> 
>>> src/XExtInt.c |2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>> 
>>> diff --git a/src/XExtInt.c b/src/XExtInt.c
>>> index d1451cc..134ccc6 100644
>>> --- a/src/XExtInt.c
>>> +++ b/src/XExtInt.c
>>> @@ -1259,7 +1259,7 @@ copyRawEvent(XGenericEventCookie *cookie_in,
>>>len = sizeof(XIRawEvent) + in->valuators.mask_len;
>>>len += bits * sizeof(double) * 2;
>>> 
>>> -ptr = cookie_out->data = malloc(sizeof(XIRawEvent));
>>> +ptr = cookie_out->data = malloc(len);
>>>if (!ptr)
>>>return False;
>>> 
>>> -- 
>>> 1.7.4.4
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 1/2] Use XORG_COMPILER_BRAND from util-macros 1.14 to check for SUNCC

2011-05-02 Thread Jeremy Huddleston

On May 2, 2011, at 10:25 PM, Peter Hutterer wrote:
> Reviewed-by: Peter Hutterer  
> (I'd obviously prefer if this patch is held back until the macros are
> released)

Ditto on that.

In fact, should we just move xorg-tls.m4 into util-macros?


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libXi] Allocate enough memory for raw events + extra data.

2011-05-02 Thread Jeremy Huddleston

On May 2, 2011, at 11:00 PM, Jeremy Huddleston wrote:
> On May 2, 2011, at 10:00 PM, Peter Hutterer wrote:
>> 
>> re "Idempotent operation"
>> I'm staring hard at those but don't really see the issue.
>> SetReqLen(req, foo, foo); is valid and for any foo that is non-zero
>> shouldn't have the effect of "Assigned value is always the same as the
>> existing value". SetReqLen essentially does req->length += foo.
> 
> Yeah... "within expansion of macro..." results are a bit bad with clang.  I 
> just opened a bug about that.  Hopefully future analytics will be better.  
> For now, at worse it's a dead assignment, and we can come back to it when 
> clang gets better at printing those results.

Ah, I see it.  There's a "len = len" in the expression.  

I dislike macros.  Why don't we make all those inline functions instead?  That 
is so much cleaner.

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libXi] Allocate enough memory for raw events + extra data.

2011-05-02 Thread Jeremy Huddleston

On May 2, 2011, at 11:24 PM, Peter Hutterer wrote:

> On Mon, May 02, 2011 at 11:11:26PM -0700, Jeremy Huddleston wrote:
>> 
>> On May 2, 2011, at 11:00 PM, Jeremy Huddleston wrote:
>>> On May 2, 2011, at 10:00 PM, Peter Hutterer wrote:
>>>> 
>>>> re "Idempotent operation"
>>>> I'm staring hard at those but don't really see the issue.
>>>> SetReqLen(req, foo, foo); is valid and for any foo that is non-zero
>>>> shouldn't have the effect of "Assigned value is always the same as the
>>>> existing value". SetReqLen essentially does req->length += foo.
>>> 
>>> Yeah... "within expansion of macro..." results are a bit bad with clang.  I 
>>> just opened a bug about that.  Hopefully future analytics will be better.  
>>> For now, at worse it's a dead assignment, and we can come back to it when 
>>> clang gets better at printing those results.
>> 
>> Ah, I see it.  There's a "len = len" in the expression.  
>> 
>> I dislike macros.  Why don't we make all those inline functions instead?  
>> That is so much cleaner.
> 
> if you want to touch Xlib macros, be my guest ;)

I think I'll pick off lower hanging fruit first.  Maybe someone else (GSOC ??) 
might want to clean up all that API.



___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libXi] Allocate enough memory for raw events + extra data.

2011-05-03 Thread Jeremy Huddleston
Ok, then I'll just 
On May 3, 2011, at 8:59 AM, Alan Coopersmith wrote:

> On 05/ 2/11 11:00 PM, Jeremy Huddleston wrote:
>>> http://people.freedesktop.org/~jeremyhu/analyzer/yuffie/20110429-1617/libXi/report-yDDyYj.html#EndPath
>>> "undefined allocation of 0 bytes" would be a server bug afaict
>> 
>> I dunno... is there anywhere that we guarantee that Xmalloc(0) returns NULL? 
>>  If so, then the macro should probably be ((size) > 0 ? malloc(size) : NULL) 
>> ... hopefully there is nothing actually using side-effects inside of 
>> Xmalloc(...) ...
> 
> We guarantee Xmalloc(0) returns non-NULL - from :

Ok, addressed in Xlib.  That should clear up a number of false positives.


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH] Don't call pixman_disable_out_of_bounds_workaround() anymore.

2011-05-03 Thread Jeremy Huddleston
Seems simple enough.

Is pixman_disable_out_of_bounds_workaround now just a no-op, or was the symbol 
removed from pixman?  Hopefully the former, but I wanted to make sure...

Reviewed-by: Jeremy Huddleston 

On May 3, 2011, at 3:36 AM, Søren Sandmann wrote:

> From: Søren Sandmann Pedersen 
> 
> Pixman used to have a workaround for a bug in old X servers, and this
> function was used to disable that workaround in servers known to be
> fixed.
> 
> In current versions of pixman, including the version the X server
> depends on, the workaround doesn't exist anymore, so there is no point
> disabling it.
> 
> Signed-off-by: Søren Sandmann 
> ---
> dix/main.c |2 --
> 1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/dix/main.c b/dix/main.c
> index 31e2d48..702176d 100644
> --- a/dix/main.c
> +++ b/dix/main.c
> @@ -139,8 +139,6 @@ int main(int argc, char *argv[], char *envp[])
> 
> InitRegions();
> 
> -pixman_disable_out_of_bounds_workaround();
> -
> CheckUserParameters(argc, argv, envp);
> 
> CheckUserAuthorization();
> -- 
> 1.7.4
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PULL 1.10] input fixes

2011-05-03 Thread Jeremy Huddleston
b4455b1..be3f41d  server-1.10-branch -> server-1.10-branch

What is the status of the floating/reattachment button release issue:

http://bugs.freedesktop.org/show_bug.cgi?id=34182
http://bugs.freedesktop.org/show_bug.cgi?id=36146

On May 3, 2011, at 17:22, Peter Hutterer wrote:

> A few input fixes for 1.10, including the valuator alignment issue that
> bothered the 1.10 series now fixed. A few passive grab fixes that affect
> clients and a minor typo fix in the accel code.
> 
> The following changes since commit b4455b119cb55cc5f61add0c25863afe8cb06484:
> 
>  XQuartz: BuildFix to build correctly with XPLUGIN_VERSION < 4 (2011-04-28 
> 13:36:23 -0700)
> 
> are available in the git repository at:
>  git://people.freedesktop.org/~whot/xserver.git server-1.10-input
> 
> Peter Hutterer (6):
>  Xi: fix valuator alignment in DeepCopyDeviceClasses (#36119)
>  Xi: return the bad device ID if a passive grab fails with BadDevice.
>  Xi: fix reply swapping function check for XIPassiveGrabDevice
>  Xi: exit with error value if CheckGrabValues failed.
>  Xi: don't swap the status byte in the XIPassiveGrab replies
>  dix: fix typo in direction calculation
> 
> Xi/exevents.c  |   11 ---
> Xi/extinit.c   |2 +-
> Xi/xipassivegrab.c |   16 
> dix/devices.c  |   47 +--
> dix/ptrveloc.c |2 +-
> include/input.h|5 +
> include/inputstr.h |2 +-
> test/input.c   |   24 +++-
> 8 files changed, 84 insertions(+), 25 deletions(-)
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH resend] input: Only release SD buttons for explicit floating/reattachment (#36146)

2011-05-03 Thread Jeremy Huddleston

Reviewed-by: Jeremy Huddleston 

On May 3, 2011, at 17:12, Peter Hutterer wrote:

> Grabbing an SD device temporary floats the device but we must not release
> the buttons. Introduced in
> 
>commit 9d23459415b84606ee4f38bb2d19054c432c8552
>Author: Peter Hutterer 
>Date:   Fri Feb 25 11:08:19 2011 +1000
> 
>dix: release all buttons and keys before reattaching a device (#34182)
> 
> X.Org Bug 36146 <http://bugs.freedesktop.org/show_bug.cgi?id=36146>
> 
> Signed-off-by: Peter Hutterer 
> ---
> Xi/xichangehierarchy.c |2 ++
> dix/devices.c  |4 +---
> include/input.h|2 ++
> 3 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/Xi/xichangehierarchy.c b/Xi/xichangehierarchy.c
> index 0736a5a..96ead6f 100644
> --- a/Xi/xichangehierarchy.c
> +++ b/Xi/xichangehierarchy.c
> @@ -355,6 +355,7 @@ detach_slave(ClientPtr client, xXIDetachSlaveInfo *c, int 
> flags[MAXDEVICES])
> goto unwind;
> }
> 
> +ReleaseButtonsAndKeys(dev);
> AttachDevice(client, dev, NULL);
> flags[dev->id] |= XISlaveDetached;
> 
> @@ -406,6 +407,7 @@ attach_slave(ClientPtr client, xXIAttachSlaveInfo *c,
> goto unwind;
> }
> 
> +ReleaseButtonsAndKeys(dev);
> AttachDevice(client, dev, newmaster);
> flags[dev->id] |= XISlaveAttached;
> 
> diff --git a/dix/devices.c b/dix/devices.c
> index 9d4fda2..9a4498b 100644
> --- a/dix/devices.c
> +++ b/dix/devices.c
> @@ -2365,7 +2365,7 @@ RecalculateMasterButtons(DeviceIntPtr slave)
>  * Generate release events for all keys/button currently down on this
>  * device.
>  */
> -static void
> +void
> ReleaseButtonsAndKeys(DeviceIntPtr dev)
> {
> InternalEvent*  eventlist = InitEventList(GetMaximumEventsNum());
> @@ -2434,8 +2434,6 @@ AttachDevice(ClientPtr client, DeviceIntPtr dev, 
> DeviceIntPtr master)
> free(dev->spriteInfo->sprite);
> }
> 
> -ReleaseButtonsAndKeys(dev);
> -
> oldmaster = GetMaster(dev, MASTER_ATTACHED);
> dev->master = master;
> 
> diff --git a/include/input.h b/include/input.h
> index 81c9dfb..1c0065f 100644
> --- a/include/input.h
> +++ b/include/input.h
> @@ -501,6 +501,8 @@ extern _X_EXPORT int GetMotionHistory(
> ScreenPtr pScreen,
> BOOL core);
> 
> +extern void ReleaseButtonsAndKeys(DeviceIntPtr dev);
> +
> extern int AttachDevice(ClientPtr client,
> DeviceIntPtr slave,
> DeviceIntPtr master);
> -- 
> 1.7.4.4
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PULL updated] XQuartz: clang, rwlocks, and one more legacy support

2011-05-04 Thread Jeremy Huddleston
The following changes since commit 5cb31cd0cbf83fff5f17a475e7b0e45246b19bf3:

  Merge remote-tracking branch 'jturney/remove-opengl-spec-download' 
(2011-04-29 09:59:49 -0700)

are available in the git repository at:

  git://people.freedesktop.org/~jeremyhu/xserver master

Jeremy Huddleston (4):
  XQuartz: Use a rwlock instead of a mutex to protect window_hash in the 
pthread case
  XQuartz: Fix incorrect typedefs with XPLUGIN_VERSION < 4
  XQuartz: Ensure that {CF,NS}_RETURNS{,_NOT}_RETAINED are defined
  XQuartz: prefs_copy_url and prefs_get_copy return retained objects

 hw/xquartz/X11Application.h |3 ++-
 hw/xquartz/applewmExt.h |6 +++---
 hw/xquartz/sanitizedCocoa.h |   39 ++-
 hw/xquartz/xpr/xprFrame.c   |   20 ++--
 4 files changed, 53 insertions(+), 15 deletions(-)

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH libX11 0/3] some static analysis fixups

2011-05-04 Thread Jeremy Huddleston
My brain-dead self accidentally git push'd these already, but please still 
review in case it needs to be changed.


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH libX11 1/3] Annotate _XIOError as _X_NORETURN

2011-05-04 Thread Jeremy Huddleston

Found-by: clang static analyzer
Signed-off-by: Jeremy Huddleston 
---
 configure.ac  |2 +-
 include/X11/Xlibint.h |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index ce86c93..a39ab8d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -77,7 +77,7 @@ AM_CONDITIONAL(HAVE_PERL, test x$PERL != xno)
 # Checks for pkg-config packages
 
 # Always required
-X11_REQUIRES='xproto >= 7.0.13 xextproto xtrans xcb >= 1.1.92'
+X11_REQUIRES='xproto >= 7.0.17 xextproto xtrans xcb >= 1.1.92'
 X11_EXTRA_DEPS="xcb >= 1.1.92"
 
 PKG_PROG_PKG_CONFIG()
diff --git a/include/X11/Xlibint.h b/include/X11/Xlibint.h
index d199a8b..3266e0d 100644
--- a/include/X11/Xlibint.h
+++ b/include/X11/Xlibint.h
@@ -899,7 +899,7 @@ extern int _XError(
 );
 extern int _XIOError(
 Display*   /* dpy */
-);
+) _X_NORETURN;
 extern int (*_XIOErrorFunction)(
 Display*   /* dpy */
 );
-- 
1.7.4.1


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH libX11 3/3] Dead code removal

2011-05-04 Thread Jeremy Huddleston

XKBGeom.c:118:27: warning: Assigned value is always the same as the existing 
value
for (key=row->keys,pos=k=0;knum_keys;k++,key++) {
   ~~~^~~~
XKBGeom.c:115:5: warning: Value stored to 'pos' is never read
pos= 0;
^~

Found-by: clang static analyzer
Signed-off-by: Jeremy Huddleston 
---
 src/xkb/XKBGeom.c |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/src/xkb/XKBGeom.c b/src/xkb/XKBGeom.c
index 2365f48..71a7d4a 100644
--- a/src/xkb/XKBGeom.c
+++ b/src/xkb/XKBGeom.c
@@ -112,7 +112,6 @@ XkbBoundsPtrbounds,sbounds;
 
 if ((!geom)||(!section)||(!row))
return False;
-pos= 0;
 bounds= &row->bounds;
 bzero(bounds,sizeof(XkbBoundsRec));
 for (key=row->keys,pos=k=0;knum_keys;k++,key++) {
-- 
1.7.4.1


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH libX11 2/3] XKB: Avoid a possible NULL dereference

2011-05-04 Thread Jeremy Huddleston

XKBGeom.c:191:25: warning: Access to field 'x1' results in a dereference of a 
null pointer (loaded from variable 'rbounds')
_XkbCheckBounds(bounds,rbounds->x1,rbounds->y1);
   ^~~

Found-by: clang static analyzer
Signed-off-by: Jeremy Huddleston 
---
 src/xkb/XKBGeom.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/xkb/XKBGeom.c b/src/xkb/XKBGeom.c
index e9e36d0..2365f48 100644
--- a/src/xkb/XKBGeom.c
+++ b/src/xkb/XKBGeom.c
@@ -147,7 +147,7 @@ register inti;
 XkbShapePtrshape;
 XkbRowPtr  row;
 XkbDoodadPtr   doodad;
-XkbBoundsPtr   bounds,rbounds=NULL;
+XkbBoundsPtr   bounds,rbounds;
 
 if ((!geom)||(!section))
return False;
@@ -186,7 +186,7 @@ XkbBoundsPtrbounds,rbounds=NULL;
default:
tbounds.x1= tbounds.x2= doodad->any.left;
tbounds.y1= tbounds.y2= doodad->any.top;
-   break;
+   continue;
}
_XkbCheckBounds(bounds,rbounds->x1,rbounds->y1);
_XkbCheckBounds(bounds,rbounds->x2,rbounds->y2);
-- 
1.7.4.1


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libX11 2/3] XKB: Avoid a possible NULL dereference

2011-05-04 Thread Jeremy Huddleston
This is the one that I especially wanted to review on the list because it 
doesn't sit well with me.

Hitting the default switch-case in the first run through the loop results in a 
NULL dereference (rbounds) when calling _XkbCheckBounds.  In subsequent runs, 
we get lucky in that rbounds was set in an earlier iteration through the loop.  
I'm not really familiar enough with XKB to know what is going on here.

Should we be doing this:
rbounds= &tbounds;

or should we just be skipping the check (which is what I accidentally pushed)?

http://people.freedesktop.org/~jeremyhu/analyzer/yuffie/20110504-/libX11/report-F0RlNS.html#EndPath

On May 4, 2011, at 11:50, Jeremy Huddleston wrote:

> 
> XKBGeom.c:191:25: warning: Access to field 'x1' results in a dereference of a 
> null pointer (loaded from variable 'rbounds')
>_XkbCheckBounds(bounds,rbounds->x1,rbounds->y1);
>   ^~~
> 
> Found-by: clang static analyzer
> Signed-off-by: Jeremy Huddleston 
> ---
> src/xkb/XKBGeom.c |4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/xkb/XKBGeom.c b/src/xkb/XKBGeom.c
> index e9e36d0..2365f48 100644
> --- a/src/xkb/XKBGeom.c
> +++ b/src/xkb/XKBGeom.c
> @@ -147,7 +147,7 @@ register int  i;
> XkbShapePtr   shape;
> XkbRowPtr row;
> XkbDoodadPtr  doodad;
> -XkbBoundsPtr bounds,rbounds=NULL;
> +XkbBoundsPtr bounds,rbounds;
> 
> if ((!geom)||(!section))
>   return False;
> @@ -186,7 +186,7 @@ XkbBoundsPtr  bounds,rbounds=NULL;
>   default:
>   tbounds.x1= tbounds.x2= doodad->any.left;
>   tbounds.y1= tbounds.y2= doodad->any.top;
> - break;
> + continue;
>   }
>   _XkbCheckBounds(bounds,rbounds->x1,rbounds->y1);
>   _XkbCheckBounds(bounds,rbounds->x2,rbounds->y2);
> -- 
> 1.7.4.1
> 
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH xinput 2/2] Initialize a few more values to defaults.

2011-05-04 Thread Jeremy Huddleston
Thanks,

Reviewed-by: Jeremy Huddleston 


On May 4, 2011, at 4:14 PM, Peter Hutterer wrote:

> If we ever print  for those, we have a buggy X server that's
> breaking the protocol. Until that happens this is just to shut up clang.
> 
> All three are assigned constant strings only, no free() needed.
> 
> Signed-off-by: Peter Hutterer 
> ---
> src/test_xi2.c |5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/test_xi2.c b/src/test_xi2.c
> index 5b56397..3c3fee3 100644
> --- a/src/test_xi2.c
> +++ b/src/test_xi2.c
> @@ -99,7 +99,7 @@ static void print_hierarchychangedevent(XIHierarchyEvent 
> *event)
> 
> for (i = 0; i < event->num_info; i++)
> {
> -char *use;
> +char *use = "";
> switch(event->info[i].use)
> {
> case XIMasterPointer: use = "master pointer"; break;
> @@ -149,7 +149,8 @@ static void print_rawevent(XIRawEvent *event)
> 
> static void print_enterleave(XILeaveEvent* event)
> {
> -char *mode, *detail;
> +char *mode = "",
> + *detail = "";
> int i;
> 
> printf("device: %d (%d)\n", event->deviceid, event->sourceid);
> -- 
> 1.7.4.4
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH xinput 1/2] Silence compiler warning

2011-05-04 Thread Jeremy Huddleston
Reviewed-by: Jeremy Huddleston 

On May 4, 2011, at 4:14 PM, Peter Hutterer wrote:

> Static analysis claims that ptr += size may assign garbage. But since the
> protocol requires format to be 8, 16 or 32, size should always have a valid
> value. Initialize to 0 to shut up clang.
> 
> Signed-off-by: Peter Hutterer 
> ---
> src/property.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/src/property.c b/src/property.c
> index f8b21c7..87f9fc6 100644
> --- a/src/property.c
> +++ b/src/property.c
> @@ -59,7 +59,7 @@ print_property(Display *dpy, XDevice* dev, Atom property)
> int act_format;
> unsigned long   nitems, bytes_after;
> unsigned char   *data, *ptr;
> -int j, done = False, size;
> +int j, done = False, size = 0;
> 
> name = XGetAtomName(dpy, property);
> printf("\t%s (%ld):\t", name, property);
> -- 
> 1.7.4.4
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [Mesa-dev] glproto changes

2011-05-04 Thread Jeremy Huddleston
Yeah... so considering the comments in mesa-dev earlier today, I was really 
surprised to see that glproto and dri2proto were tagged today.  I think we need 
to brownbag retract and rethink this.

These changes break API.  I'm all for fixing the structs, but anything that 
breaks API (or worse, protocol) certainly warrants much more than the +0.0.1 
version bump.  This also makes it impossible to build the current dev and 
current stable with the same protos installed.  I haven't looked at the details 
specifically, but I suspect that it also changes what is on the wire, meaning 
clients and the server may disagree depending on which glproto version they 
were using.  Is that the case?  If not, can't we solve this with some creative 
union{}ing?

Either way, I think we should retract the glproto 1.4.13 release until we can 
get this straightened out.

On May 4, 2011, at 18:17, Dave Airlie wrote:

> So I wasn't watching and glproto broke its interface, and I think its bad.
> 
> Why?
> 
> You can no longer bisect things across this point without now moving glproto.
> glxproto.h:xGLXBufferSwapComplete was a released header file
> definition, you cannot go back and change history.
> 
> This should have been handled with xGLXBufferSwapComplete2 struct that
> newer mesa and X server could would use
> instead of the older code. Old code would build against the old broken
> defintion but since its broken it wouldn't matter,
> and new code would build against the new fixed definition.
> 
> This doesn't address the I need the latest glproto to build mesa and
> my distro doesn't have which concerns me less
> than the I can't bisect anymore and I fully agree with Jesse that the
> last time we tried using #ifdef for this sort of thing it led
> to a number of untested combos resulting in impossible to debug issues.
> 
> Dave.
> ___
> mesa-dev mailing list
> mesa-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [Mesa-dev] glproto changes

2011-05-05 Thread Jeremy Huddleston
Why is sbc a 32bit field in xGLXBufferSwapComplete2 and 
xDRI2BufferSwapComplete2 when it is a 64bit field in GLXBufferSwapComplete?

The hunk at the bottom will result in casting a 64bit int to a 32bit int.  If 
you're going to change this, shouldn't you also change GLXBufferSwapComplete, 
DRI2SwapEvent() and everything else that has a 64bit swap count?

Isn't it easier to correct the spec to match reality?

--- a/hw/xfree86/dri2/dri2ext.c
+++ b/hw/xfree86/dri2/dri2ext.c
@@ -359,7 +359,7 @@ static void
 DRI2SwapEvent(ClientPtr client, void *data, int type, CARD64 ust, CARD64 msc,
  CARD64 sbc)
 {
-xDRI2BufferSwapComplete event;
+xDRI2BufferSwapComplete2 event;
 DrawablePtr pDrawable = data;
 
 event.type = DRI2EventBase + DRI2_BufferSwapComplete;
@@ -369,8 +369,7 @@ DRI2SwapEvent(ClientPtr client, void *data, int type, 
CARD64 ust, CARD64 msc,
 event.ust_lo = ust & 0x;
 event.msc_hi = (CARD64)msc >> 32;
 event.msc_lo = msc & 0x;
-event.sbc_hi = (CARD64)sbc >> 32;
-event.sbc_lo = sbc & 0x;
+event.sbc = sbc;
 
 WriteEventsToClient(client, 1, (xEvent *)&event);
 }



On May 5, 2011, at 08:28, Jesse Barnes wrote:

> On Wed, 4 May 2011 21:29:23 -0700
> Jeremy Huddleston  wrote:
> 
>> Yeah... so considering the comments in mesa-dev earlier today, I was really 
>> surprised to see that glproto and dri2proto were tagged today.  I think we 
>> need to brownbag retract and rethink this.
> 
> Damnit; right when Dave mentioned this last night I knew I had gone too
> far in trying to make sure the fix was propagated.  I hate it when he's
> right!
> 
> Yeah, having a rule that we can't touch existing proto structs makes
> sense unless we want to do a major break.  It certainly makes pushing
> out updates eaiser and preserves bisection...
> 
>> These changes break API.  I'm all for fixing the structs, but anything that 
>> breaks API (or worse, protocol) certainly warrants much more than the +0.0.1 
>> version bump.  This also makes it impossible to build the current dev and 
>> current stable with the same protos installed.  I haven't looked at the 
>> details specifically, but I suspect that it also changes what is on the 
>> wire, meaning clients and the server may disagree depending on which glproto 
>> version they were using.  Is that the case?  If not, can't we solve this 
>> with some creative union{}ing?
> 
> In this case, what's on the wire is pretty much the same; if the client
> and server don't match, you may get a different kind of corruption in
> the affected field (0 vs some other value), but things are no worse.
> 
>> Either way, I think we should retract the glproto 1.4.13 release until we 
>> can get this straightened out.
> 
> Ok; fwiw my rationale was twofold:
>  1) make sure master requires the new proto headers to avoid some of
> the trouble we've had in the past with ifdefs and untested paths
> (though again, the failure mode is benign in this case)
>  2) make the proto breakage obvious even for older code; the fix is
> trivial
> 
> But I guess (2) is a bit much... after pushing I started having
> nightmares about the input proto changes from awhile back.
> 
> Here are what the backwards compatible changes look like...  Look
> better?
> 
> Thanks,
> -- 
> Jesse "Breaker of Builds" Barnes
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH] xserver/glx/dri2: use new GLX/DRI2 swap event types

2011-05-05 Thread Jeremy Huddleston
For *proto/mesa:
Reviewed-by: Jeremy Huddleston 

For xserver:
This looks incomplete.  You also need to update swap_count in DRI2DrawableRec 
to be CARD32.


On May 5, 2011, at 12:45 PM, Jesse Barnes wrote:

> Use the new event types so we can pass a valid SBC value to clients.
> Fix up the completion calls to use CARD32 instead of CARD64 to match
> the new field size.
> 
> Signed-off-by: Jesse Barnes 
> 
> diff --git a/configure.ac b/configure.ac
> index 6eb780c..3e0ed5d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -771,11 +771,11 @@ RECORDPROTO="recordproto >= 1.13.99.1"
> SCRNSAVERPROTO="scrnsaverproto >= 1.1"
> RESOURCEPROTO="resourceproto"
> DRIPROTO="xf86driproto >= 2.1.0"
> -DRI2PROTO="dri2proto >= 2.3"
> +DRI2PROTO="dri2proto >= 2.5"
> XINERAMAPROTO="xineramaproto"
> BIGFONTPROTO="xf86bigfontproto >= 1.2.0"
> DGAPROTO="xf86dgaproto >= 2.0.99.1"
> -GLPROTO="glproto >= 1.4.10"
> +GLPROTO="glproto >= 1.4.14"
> DMXPROTO="dmxproto >= 2.2.99.1"
> VIDMODEPROTO="xf86vidmodeproto >= 2.2.99.1"
> WINDOWSWMPROTO="windowswmproto"
> diff --git a/glx/glxdri2.c b/glx/glxdri2.c
> index 93c5e5b..e872258 100644
> --- a/glx/glxdri2.c
> +++ b/glx/glxdri2.c
> @@ -163,10 +163,10 @@ __glXDRIdrawableWaitGL(__GLXdrawable *drawable)
> 
> static void
> __glXdriSwapEvent(ClientPtr client, void *data, int type, CARD64 ust,
> -   CARD64 msc, CARD64 sbc)
> +   CARD64 msc, CARD32 sbc)
> {
> __GLXdrawable *drawable = data;
> -xGLXBufferSwapComplete wire;
> +xGLXBufferSwapComplete2 wire;
> 
> if (!(drawable->eventMask & GLX_BUFFER_SWAP_COMPLETE_INTEL_MASK))
>   return;
> @@ -192,8 +192,7 @@ __glXdriSwapEvent(ClientPtr client, void *data, int type, 
> CARD64 ust,
> wire.ust_lo = ust & 0x;
> wire.msc_hi = msc >> 32;
> wire.msc_lo = msc & 0x;
> -wire.sbc_hi = sbc >> 32;
> -wire.sbc_lo = sbc & 0x;
> +wire.sbc = sbc;
> 
> WriteEventsToClient(client, 1, (xEvent *) &wire);
> }
> diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
> index fe0bf6c..2a41ead 100644
> --- a/hw/xfree86/dri2/dri2.h
> +++ b/hw/xfree86/dri2/dri2.h
> @@ -51,7 +51,7 @@ extern CARD8 dri2_minor;
> 
> typedef DRI2BufferRec DRI2Buffer2Rec, *DRI2Buffer2Ptr;
> typedef void (*DRI2SwapEventPtr)(ClientPtr client, void *data, int type,
> -  CARD64 ust, CARD64 msc, CARD64 sbc);
> +  CARD64 ust, CARD64 msc, CARD32 sbc);
> 
> 
> typedef DRI2BufferPtr (*DRI2CreateBuffersProcPtr)(DrawablePtr pDraw,
> diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c
> index 4e48e65..552b26b 100644
> --- a/hw/xfree86/dri2/dri2ext.c
> +++ b/hw/xfree86/dri2/dri2ext.c
> @@ -357,9 +357,9 @@ vals_to_card64(CARD32 lo, CARD32 hi)
> 
> static void
> DRI2SwapEvent(ClientPtr client, void *data, int type, CARD64 ust, CARD64 msc,
> -   CARD64 sbc)
> +   CARD32 sbc)
> {
> -xDRI2BufferSwapComplete event;
> +xDRI2BufferSwapComplete2 event;
> DrawablePtr pDrawable = data;
> 
> event.type = DRI2EventBase + DRI2_BufferSwapComplete;
> @@ -369,8 +369,7 @@ DRI2SwapEvent(ClientPtr client, void *data, int type, 
> CARD64 ust, CARD64 msc,
> event.ust_lo = ust & 0x;
> event.msc_hi = (CARD64)msc >> 32;
> event.msc_lo = msc & 0x;
> -event.sbc_hi = (CARD64)sbc >> 32;
> -event.sbc_lo = sbc & 0x;
> +event.sbc = sbc;
> 
> WriteEventsToClient(client, 1, (xEvent *)&event);
> }
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH] xserver/glx/dri2: use new GLX/DRI2 swap event types

2011-05-05 Thread Jeremy Huddleston

On May 5, 2011, at 1:09 PM, Jesse Barnes wrote:

> On Thu, 05 May 2011 12:59:50 -0700
> Jeremy Huddleston  wrote:
> 
>> For *proto/mesa:
>> Reviewed-by: Jeremy Huddleston 
> 
> Thanks.
> 
>> For xserver:
>> This looks incomplete.  You also need to update swap_count in 
>> DRI2DrawableRec to be CARD32.
> 
> That value is used in other places that actually preserve the full 64
> bits.  I could add an explicit cast in the few places where we pass it
> to the swap completion though if you think that's better...

Yes, I realize that there are some areas that use 64bit for sbc.  Do we really 
need 64bits for this?  If so, how do we properly to communicate the 64bit value 
between server/client?  Currently, your changes result in an implicit cast from 
64bit to 32bits in DRI2SwapComplete which will cause problems if we really do 
need those extra bits:

if (swap_complete)
swap_complete(client, swap_data, type, ust, frame, pPriv->swap_count);

It looks to me like much more of DRI2 should be rewritten to use 32bit swap 
counts, or there should be some other mechanism in place to communicate the 
full 64bit value.


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH] xserver/glx/dri2: use new GLX/DRI2 swap event types

2011-05-05 Thread Jeremy Huddleston
>> Yes, I realize that there are some areas that use 64bit for sbc.  Do we 
>> really need 64bits for this?  If so, how do we properly to communicate the 
>> 64bit value between server/client?  Currently, your changes result in an 
>> implicit cast from 64bit to 32bits in DRI2SwapComplete which will cause 
>> problems if we really do need those extra bits:
>> 
>>if (swap_complete)
>>swap_complete(client, swap_data, type, ust, frame, pPriv->swap_count);
>> 
>> It looks to me like much more of DRI2 should be rewritten to use 32bit swap 
>> counts, or there should be some other mechanism in place to communicate the 
>> full 64bit value.
> 
> Yeah there are other mechanisms (the OML sync extension) for
> communicating the whole 64 bits.  Both the swap reply and msc reply
> deal in the full 64 bits, just the swap event (an unrelated extension)
> truncates to 32 bits.  (Also I think the 64 bit size for swap count in
> the OML extension is just inertia; getting to a count that high even at
> 120Hz is going to take awhile.)

Ok, well this all really doesn't sit well with me, but it would put my mind 
slightly at ease if you would do some logging of the problem if we encounter it 
in DRI2SwapComplete.  That might give your future self some clue as to what is 
going wrong when you see issues down the road:

   if (swap_complete) {
   if (pPriv->swap_count > 0x)
   ErrorF("something appropriate");
   swap_complete(client, swap_data, type, ust, frame, 
(CARD32)pPriv->swap_count);
   }

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH] xserver/glx/dri2: use new GLX/DRI2 swap event types

2011-05-06 Thread Jeremy Huddleston
Yeah, that looks about right.

This in combination with the latest version of "xserver/glx/dri2: use new 
GLX/DRI2 swap event types"

Reviewed-by: Jeremy Huddleston 

On May 6, 2011, at 10:39, Jesse Barnes wrote:

> On Fri, 06 May 2011 09:25:35 +0200
> Michel Dänzer  wrote:
> 
>> On Don, 2011-05-05 at 14:02 -0700, Jesse Barnes wrote: 
>>>>>   if (swap_complete) {
>>>>>   if (pPriv->swap_count > 0x)
>>>>>   ErrorF("something appropriate");
>>>>>   swap_complete(client, swap_data, type, ust, frame, 
>>>>> (CARD32)pPriv->swap_count);
>>>>>   }
>>>> 
>>>> Yeah, it's annoying.  How about I leave out the error message and handle
>>>> wrapping on the client side instead?  That way at least the client code
>>>> won't notice that the server is only transmitting 32 bits...
>>> 
>>> Nevermind, that can't work generally since clients are free to
>>> mask/unmask the events, so we could miss a 0 count and thus a wrap.
>> 
>> Missing 0 isn't a problem, you can assume there's been a wraparound
>> whenever the current value is smaller than the previous one. This would
>> only fail if the client misses several wraparounds, in which case
>> apparently it doesn't care all that much in the first place. :)
>> 
>> Please do this.
> 
> How does this look for the direct case?  The indirect case is a little
> more complicated since I need to add a new glx_drawable type and track
> it (Kristian says this should help clean up some of the Apple mess as
> well).  This patch handles at least one wrap correctly (I set the
> server's starting swap_count at 0xfff0 and then ran the
> glx-swap-event test in piglit; wrapping worked ok).
> 
> -- 
> Jesse Barnes, Intel Open Source Technology Center
> 
> From f1e288f61e10b71018600a24ca0bdda93f8481db Mon Sep 17 00:00:00 2001
> From: Jesse Barnes 
> Date: Fri, 6 May 2011 10:31:24 -0700
> Subject: [PATCH] DRI2: handle swap event swap count wrapping
> 
> Add a wrap counter to the DRI drawable and track it as we receive events.
> This allows us to support the full 64 bits of the event structure we
> pass to the client even though the server only gives us a 32 bit count.
> 
> Signed-off-by: Jesse Barnes 
> ---
> src/glx/dri2.c  |   12 +++-
> src/glx/dri2_glx.c  |2 ++
> src/glx/glxclient.h |2 ++
> 3 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/src/glx/dri2.c b/src/glx/dri2.c
> index 8654a37..08ceec0 100644
> --- a/src/glx/dri2.c
> +++ b/src/glx/dri2.c
> @@ -88,6 +88,7 @@ static Bool
> DRI2WireToEvent(Display *dpy, XEvent *event, xEvent *wire)
> {
>XExtDisplayInfo *info = DRI2FindDisplay(dpy);
> +   __GLXDRIdrawable *pdraw;
> 
>XextCheckExtension(dpy, info, dri2ExtensionName, False);
> 
> @@ -124,7 +125,16 @@ DRI2WireToEvent(Display *dpy, XEvent *event, xEvent 
> *wire)
>   }
>   aevent->ust = ((CARD64)awire->ust_hi << 32) | awire->ust_lo;
>   aevent->msc = ((CARD64)awire->msc_hi << 32) | awire->msc_lo;
> -  aevent->sbc = awire->sbc;
> +
> +  pdraw = dri2GetGlxDrawableFromXDrawableId(dpy, awire->drawable);
> +  if (!pdraw)
> +  return False;
> +
> +  if (awire->sbc < pdraw->lastEventSbc)
> +  pdraw->eventSbcWrap += 0x1;
> +  pdraw->lastEventSbc = awire->sbc;
> +  aevent->sbc = awire->sbc + pdraw->eventSbcWrap;
> +
>   return True;
>}
> #endif
> diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
> index fc0237a..421f543 100644
> --- a/src/glx/dri2_glx.c
> +++ b/src/glx/dri2_glx.c
> @@ -258,6 +258,8 @@ dri2CreateDrawable(struct glx_screen *base, XID xDrawable,
>pdraw->base.xDrawable = xDrawable;
>pdraw->base.drawable = drawable;
>pdraw->base.psc = &psc->base;
> +   pdraw->base.lastEventSbc = 0;
> +   pdraw->base.eventSbcWrap = 0;
>pdraw->bufferCount = 0;
>pdraw->swap_interval = 1; /* default may be overridden below */
>pdraw->have_back = 0;
> diff --git a/src/glx/glxclient.h b/src/glx/glxclient.h
> index 2b6966f..1eb2483 100644
> --- a/src/glx/glxclient.h
> +++ b/src/glx/glxclient.h
> @@ -138,6 +138,8 @@ struct __GLXDRIdrawableRec
>GLenum textureTarget;
>GLenum textureFormat;/* EXT_texture_from_pixmap support */
>unsigned long eventMask;
> +   uint32_t lastEventSbc;
> +   int64_t eventSbcWrap;
> };
> 
> /*
> -- 
> 1.7.4.1
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH] xserver/glx/dri2: use new GLX/DRI2 swap event types

2011-05-06 Thread Jeremy Huddleston
I believe you want to s/Xmalloc/malloc/

Yes, I can't speak for all the internals of DRI2 since most of that is foreign 
to me, but from a high level, this looks like the right approach.  As for your 
specific question about Apple stuff, it's been a while since I touched that... 
perhaps something we can have a sit-down about during XDC.  So from a 
high-level point of view:

Reviewed-by: Jeremy Huddleston 

but this should also be reviewed by someone who has more understanding of GLX.

--Jeremy

On May 6, 2011, at 14:01, Jesse Barnes wrote:

> On Fri, 6 May 2011 13:00:19 -0700
> Jeremy Huddleston  wrote:
> 
>> Yeah, that looks about right.
>> 
>> This in combination with the latest version of "xserver/glx/dri2: use new 
>> GLX/DRI2 swap event types"
>> 
>> Reviewed-by: Jeremy Huddleston 
> 
> Ok here's a more complete patch.  It touches GLX and involves drawable
> lifetimes, which I'm not that familiar with, so careful review
> appreciated.  Note the X vs GLX drawable ID switching in the DRI2 event
> handler (DRI2 just deals with X IDs).
> 
> Kristian and Jeremy, is this a good basis for moving the Apple stuff
> over to a client GLX drawable type?
> 
> -- 
> Jesse Barnes, Intel Open Source Technology Center
> 
> From fae63609dd4fd20ccd84d2211787136bb9a1da05 Mon Sep 17 00:00:00 2001
> From: Jesse Barnes 
> Date: Fri, 6 May 2011 10:31:24 -0700
> Subject: [PATCH] GLX/DRI2: handle swap event swap count wrapping
> 
> Create a new GLX drawable struct to track client related info, and add a
> wrap counter to it drawable and track it as we receive events.  This
> allows us to support the full 64 bits of the event structure we pass to
> the client even though the server only gives us a 32 bit count.
> 
> Signed-off-by: Jesse Barnes 
> ---
> src/glx/dri2.c|   12 +-
> src/glx/glx_pbuffer.c |   14 
> src/glx/glxclient.h   |   16 +
> src/glx/glxcmds.c |   57 +
> src/glx/glxext.c  |   16 -
> 5 files changed, 112 insertions(+), 3 deletions(-)
> 
> diff --git a/src/glx/dri2.c b/src/glx/dri2.c
> index 8654a37..229840d 100644
> --- a/src/glx/dri2.c
> +++ b/src/glx/dri2.c
> @@ -88,6 +88,7 @@ static Bool
> DRI2WireToEvent(Display *dpy, XEvent *event, xEvent *wire)
> {
>XExtDisplayInfo *info = DRI2FindDisplay(dpy);
> +   struct glx_drawable *glxDraw;
> 
>XextCheckExtension(dpy, info, dri2ExtensionName, False);
> 
> @@ -98,6 +99,9 @@ DRI2WireToEvent(Display *dpy, XEvent *event, xEvent *wire)
>{
>   GLXBufferSwapComplete *aevent = (GLXBufferSwapComplete *)event;
>   xDRI2BufferSwapComplete2 *awire = (xDRI2BufferSwapComplete2 *)wire;
> +  __GLXDRIdrawable *pdraw;
> +
> +  pdraw = dri2GetGlxDrawableFromXDrawableId(dpy, awire->drawable);
> 
>   /* Ignore swap events if we're not looking for them */
>   aevent->type = dri2GetSwapEventType(dpy, awire->drawable);
> @@ -124,7 +128,13 @@ DRI2WireToEvent(Display *dpy, XEvent *event, xEvent 
> *wire)
>   }
>   aevent->ust = ((CARD64)awire->ust_hi << 32) | awire->ust_lo;
>   aevent->msc = ((CARD64)awire->msc_hi << 32) | awire->msc_lo;
> -  aevent->sbc = awire->sbc;
> +
> +  glxDraw = GetGLXDrawable(dpy, pdraw->drawable);
> +  if (awire->sbc < glxDraw->lastEventSbc)
> +  glxDraw->eventSbcWrap += 0x1;
> +  glxDraw->lastEventSbc = awire->sbc;
> +  aevent->sbc = awire->sbc + glxDraw->eventSbcWrap;
> +
>   return True;
>}
> #endif
> diff --git a/src/glx/glx_pbuffer.c b/src/glx/glx_pbuffer.c
> index ec54f1e..420e754 100644
> --- a/src/glx/glx_pbuffer.c
> +++ b/src/glx/glx_pbuffer.c
> @@ -380,7 +380,9 @@ static GLXDrawable
> CreateDrawable(Display *dpy, struct glx_config *config,
>Drawable drawable, const int *attrib_list, CARD8 glxCode)
> {
> +   struct glx_display *const priv = __glXInitialize(dpy);
>xGLXCreateWindowReq *req;
> +   struct glx_drawable *glxDraw;
>CARD32 *data;
>unsigned int i;
>CARD8 opcode;
> @@ -395,6 +397,10 @@ CreateDrawable(Display *dpy, struct glx_config *config,
>if (!opcode)
>   return None;
> 
> +   glxDraw = Xmalloc(sizeof(*glxDraw));
> +   if (!glxDraw)
> +  return None;
> +
>LockDisplay(dpy);
>GetReqExtra(GLXCreateWindow, 8 * i, req);
>data = (CARD32 *) (req + 1);
> @@ -413,6 +419,11 @@ CreateDrawable(Display *dpy, struct glx_config *config,
>UnlockDisplay(dpy);
>SyncHandle();
> 
> +   if (InitGLXDrawable(dpy, glxDraw, drawable, req->glxwindow)) {
> +  

[PATCH rendercheck] Report the success_mask to stdout

2011-05-07 Thread Jeremy Huddleston

This was previously computed but never passed on to the caller.

Found-by: clang static analyzer
Signed-off-by: Jeremy Huddleston 
---
 tests.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tests.c b/tests.c
index f722bc6..1b47eee 100644
--- a/tests.c
+++ b/tests.c
@@ -752,7 +752,7 @@ do {
\
free(test_mask);
free(test_dst);
 
-   printf("%d tests passed of %d total\n", tests_passed, tests_total);
+   printf("%d tests passed of %d total (successful groups: 0x%x)\n", 
tests_passed, tests_total, success_mask);
 
return tests_passed == tests_total;
 }
-- 
1.7.4.1
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH iceauth] Error out and avoid a call to malloc(0) if given a bad hex string

2011-05-08 Thread Jeremy Huddleston
Yep.  Scroll up in your mail log ;)

http://cgit.freedesktop.org/xorg/app/xauth/commit/?id=5032c286df16737277c9a04e1083171ffec89000

On May 7, 2011, at 11:13 PM, Alan Coopersmith wrote:

> On 04/28/11 12:53 AM, Jeremy Huddleston wrote:
>> 
>> Found-by: clang static analyzer
>> Signed-off-by: Jeremy Huddleston 
>> ---
>> process.c |4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/process.c b/process.c
>> index f51e643..56b7aaf 100644
>> --- a/process.c
>> +++ b/process.c
>> @@ -401,8 +401,8 @@ static int cvthexkey (   /* turn hex key string into 
>> octets */
>>  len++;
>> }
>> 
>> -/* if odd then there was an error */
>> -if ((len & 1) == 1) return -1;
>> +/* if 0 or odd, then there was an error */
>> +if (len == 0 || (len & 1) == 1) return -1;
>> 
>> 
>> /* now we know that the input is good */
> 
> Looks like xauth needs the same fix.  (iceauth is mostly a
> duplicate copy of xauth.)
> 
> -- 
>   -Alan Coopersmith-alan.coopersm...@oracle.com
>Oracle Solaris Platform Engineering: X Window System
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH xinput 1/2] Silence compiler warning

2011-05-08 Thread Jeremy Huddleston
Yeah, but Peter's patch allows xinput to keep chugging along, reporting what it 
sees (invalid as it may be from a buggy server).  I think that behavior is 
better than jumping ship at the first sign of trouble.


On May 8, 2011, at 6:07 AM, Simon Thum wrote:

> On 05/05/2011 01:14 AM, Peter Hutterer wrote:
>> Static analysis claims that ptr += size may assign garbage. But since the
>> protocol requires format to be 8, 16 or 32, size should always have a valid
>> value. Initialize to 0 to shut up clang.
> Perhaps it would be preferable to have something like:
> 
> switch(format) {
> //...8,16,32
> default: print("Sunken ship"); exit(-1);
> }
> 
> If exit is __noreturn__, clang should shut up as well.
> 
> Cheers,
> 
> Simon
> 
> 
>> 
>> Signed-off-by: Peter Hutterer 
>> ---
>> src/property.c |2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/src/property.c b/src/property.c
>> index f8b21c7..87f9fc6 100644
>> --- a/src/property.c
>> +++ b/src/property.c
>> @@ -59,7 +59,7 @@ print_property(Display *dpy, XDevice* dev, Atom property)
>> int act_format;
>> unsigned long   nitems, bytes_after;
>> unsigned char   *data, *ptr;
>> -int j, done = False, size;
>> +int j, done = False, size = 0;
>> 
>> name = XGetAtomName(dpy, property);
>> printf("\t%s (%ld):\t", name, property);
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH xauth/iceauth] auth_finalize: Attempt to rename() if link() fails

2011-05-08 Thread Jeremy Huddleston

On some file systems (like AFP), hard links are not supported. If
link fails, try rename() before giving up.

Reported-by: Jamie Kennea 
Signed-off-by: Jeremy Huddleston 
---
 process.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/process.c b/process.c
index 04abc33..a4021c7 100644
--- a/process.c
+++ b/process.c
@@ -888,7 +888,8 @@ auth_finalize(void)
 #if defined(WIN32) || defined(__UNIXOS2__)
if (rename(temp_name, xauth_filename) == -1)
 #else
-   if (link (temp_name, xauth_filename) == -1)
+   /* Attempt to rename() if link() fails, since this may be on a 
FS that does not support hard links */
+   if (link (temp_name, xauth_filename) == -1 && rename(temp_name, 
xauth_filename) == -1)
 #endif
{
fprintf (stderr,
-- 
1.7.4.4


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH xrandr] find_mode: Search for the mode closes to the specified rate

2011-05-09 Thread Jeremy Huddleston
Hey Keith,

clang found an issue in xrandr that I'd like you to review the fix for.  I'm 
fairly certain this is what you intended.

Thanks,
Jeremy

For reference:
http://people.freedesktop.org/~jeremyhu/analyzer/yuffie/20110509-/xrandr/report-gZOzCf.html#EndPath
http://cgit.freedesktop.org/xorg/app/xrandr/commit/?id=d9aeb4a7544ad4a33f6f54bc46bff5cdf231a986

---

>From 9f1eb72eea2ba5375be9a2db1e193cda8a50a841 Mon Sep 17 00:00:00 2001
From: Jeremy Huddleston 
Date: Mon, 9 May 2011 09:21:29 -0700
Subject: [PATCH] find_mode: Search for the mode closes to the specified rate

This was the intention of d9aeb4a7544ad4a33f6f54bc46bff5cdf231a986, but
find_mode was still picking the first string match rather than the
match with the closest refresh rate.

xrandr.c:740:3: warning: Value stored to 'bestDist' is never read
bestDist = dist;
^  

Found-by: clang static analyzer
Signed-off-by: Jeremy Huddleston 
---
 xrandr.c |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/xrandr.c b/xrandr.c
index 564d4da..10d034d 100644
--- a/xrandr.c
+++ b/xrandr.c
@@ -740,7 +740,6 @@ find_mode (name_t *name, double refresh)
bestDist = dist;
best = mode;
}
-   break;
}
 }
 return best;
-- 
1.7.4.4


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH rendercheck] Report the success_mask to stdout

2011-05-09 Thread Jeremy Huddleston
Well it's better than nothing... but ok, I'll beautify it.

On May 9, 2011, at 13:47, Eric Anholt wrote:

> On Sat, 7 May 2011 18:20:17 -0700, Jeremy Huddleston  
> wrote:
>> 
>> This was previously computed but never passed on to the caller.
>> 
>> Found-by: clang static analyzer
>> Signed-off-by: Jeremy Huddleston 
> 
> Doing something useful with those success flags would be nice, but I
> don't think a hex dump of it is that.
> 
>> ---
>> tests.c |2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/tests.c b/tests.c
>> index f722bc6..1b47eee 100644
>> --- a/tests.c
>> +++ b/tests.c
>> @@ -752,7 +752,7 @@ do { 
>> \
>>  free(test_mask);
>>  free(test_dst);
>> 
>> -printf("%d tests passed of %d total\n", tests_passed, tests_total);
>> +printf("%d tests passed of %d total (successful groups: 0x%x)\n", 
>> tests_passed, tests_total, success_mask);
>> 
>>  return tests_passed == tests_total;
>> }
>> -- 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH rendercheck] Report which test groups passed successfully

2011-05-09 Thread Jeremy Huddleston

This was previously computed but never passed on to the caller.

Found-by: clang static analyzer
Signed-off-by: Jeremy Huddleston 
---
 configure.ac  |2 +-
 main.c|   83 +++--
 rendercheck.h |5 +++-
 tests.c   |2 +
 4 files changed, 58 insertions(+), 34 deletions(-)

diff --git a/configure.ac b/configure.ac
index 4fa5a63..09b07bd 100644
--- a/configure.ac
+++ b/configure.ac
@@ -22,7 +22,7 @@ XORG_DEFAULT_OPTIONS
 AC_CHECK_HEADERS([err.h])
 
 # Checks for pkg-config packages
-PKG_CHECK_MODULES(RC, [xrender x11])
+PKG_CHECK_MODULES(RC, [xrender x11 xproto >= 7.0.17])
 
 AC_CONFIG_FILES([Makefile
  man/Makefile])
diff --git a/main.c b/main.c
index 9a97d72..7fa771c 100644
--- a/main.c
+++ b/main.c
@@ -103,16 +103,55 @@ describe_format(char *desc, int len, XRenderPictFormat 
*format)
}
 }
 
+struct {
+int flag;
+const char *name;
+} available_tests[] = {
+{TEST_FILL, "fill"},
+{TEST_DSTCOORDS, "dcoords"},
+{TEST_SRCCOORDS, "scoords"},
+{TEST_MASKCOORDS, "mcoords"},
+{TEST_TSRCCOORDS, "tscoords"},
+{TEST_TMASKCOORDS, "tmcoords"},
+{TEST_BLEND, "blend"},
+{TEST_COMPOSITE, "composite"},
+{TEST_CACOMPOSITE, "cacomposite"},
+{TEST_GRADIENTS, "gradients"},
+{TEST_REPEAT, "repeat"},
+{TEST_TRIANGLES, "triangles"},
+{TEST_BUG7366, "bug7366"},
+{0, NULL}
+};
+
+void print_tests(FILE *file, int tests) {
+int i, j;
+
+for(i=0, j=0; available_tests[i].name; i++) {
+if(!(available_tests[i].flag & tests))
+continue;
+if(j % 5 == 0) {
+if(j != 0)
+putc('\n', stderr);
+putc('\t', stderr);
+} else {
+fprintf(stderr, ", ");
+}
+fprintf(stderr, "%s", available_tests[i].name);
+j++;
+}
+if(j)
+fprintf(file, "\n");
+}
+
+_X_NORETURN
 static void
 usage (char *program)
 {
 fprintf(stderr, "usage: %s [-d|--display display] [-v|--verbose]\n"
"\t[-t test1,test2,...] [-o op1,op2,...] [-f format1,format2,...]\n"
"\t[--sync] [--minimalrendering] [--version]\n"
-"\tAvailable tests: fill,dcoords,scoords,mcoords,tscoords,\n"
-
"\t\ttmcoords,blend,composite,cacomposite,gradients,repeat,triangles,\n"
-"\t\tbug7366\n",
-   program);
+   "Available tests:\n", program);
+print_tests(stderr, ~0);
 exit(1);
 }
 
@@ -197,35 +236,15 @@ int main(int argc, char **argv)
enabled_tests = 0;
 
while ((test = strsep(&nextname, ",")) != NULL) {
-   if (strcmp(test, "fill") == 0) {
-   enabled_tests |= TEST_FILL;
-   } else if (strcmp(test, "dcoords") == 0) {
-   enabled_tests |= TEST_DSTCOORDS;
-   } else if (strcmp(test, "scoords") == 0) {
-   enabled_tests |= TEST_SRCCOORDS;
-   } else if (strcmp(test, "mcoords") == 0) {
-   enabled_tests |= TEST_MASKCOORDS;
-   } else if (strcmp(test, "tscoords") == 0) {
-   enabled_tests |= TEST_TSRCCOORDS;
-   } else if (strcmp(test, "tmcoords") == 0) {
-   enabled_tests |= TEST_TMASKCOORDS;
-   } else if (strcmp(test, "blend") == 0) {
-   enabled_tests |= TEST_BLEND;
-   } else if (strcmp(test, "composite") == 0) {
-   enabled_tests |= TEST_COMPOSITE;
-   } else if (strcmp(test, "cacomposite") == 0) {
-   enabled_tests |= TEST_CACOMPOSITE;
-   } else if (strcmp(test, "gradients") == 0) {
-   enabled_tests |= TEST_GRADIENTS;
-   } else if (strcmp(test, "repeat") == 0) {
-   enabled_tests |= TEST_REPEAT;
-   } else if (strcmp(test, "triangles") == 0) {
-   enabled_tests |= TEST_TRIANGLES;
-   } else if (strcmp(test, "bug7366") == 0) {
-   enabled_tests |= T

Re: [PATCH] include: export GetProximityEvents and QueueProximityEvents

2011-05-10 Thread Jeremy Huddleston
Reviewed-by: Jeremy Huddleston 

On May 10, 2011, at 17:38, Peter Hutterer wrote:

> This is mainly needed for consistency with GetPointerEvents and friend.
> No-one seems to actually need this function from outside the usual DDXs.
> 
> Signed-off-by: Peter Hutterer 
> ---
> This obviously goes on top of the Queue***Events series in this thread.
> 
> include/input.h |4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/input.h b/include/input.h
> index c1783f7..81c9dfb 100644
> --- a/include/input.h
> +++ b/include/input.h
> @@ -469,13 +469,13 @@ extern _X_EXPORT void QueueKeyboardEvents(
> int key_code,
> const ValuatorMask *mask);
> 
> -extern int GetProximityEvents(
> +extern _X_EXPORT int GetProximityEvents(
> InternalEvent *events,
> DeviceIntPtr pDev,
> int type,
> const ValuatorMask *mask);
> 
> -extern void QueueProximityEvents(
> +extern _X_EXPORT void QueueProximityEvents(
> DeviceIntPtr pDev,
> int type,
> const ValuatorMask *mask);
> -- 
> 1.7.4.4
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH twm 1/3] Address a possible NULL pointer dereference

2011-05-11 Thread Jeremy Huddleston

menus.c:523:24: warning: Access to field 'w' results in a dereference of a null 
pointer (loaded from variable 'ActiveMenu')
if (XFindContext(dpy, ActiveMenu->w, ScreenContext, &context_data) == 0)
  ^~

Found-by: clang static analyzer
Signed-off-by: Jeremy Huddleston 
---
 src/menus.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/src/menus.c b/src/menus.c
index bc688e6..34a3c8e 100644
--- a/src/menus.c
+++ b/src/menus.c
@@ -511,13 +511,16 @@ UpdateMenu()
 
if (Event.type != MotionNotify)
continue;
+ 
+   if (!ActiveMenu)
+continue;
 
done = FALSE;
XQueryPointer( dpy, ActiveMenu->w, &JunkRoot, &JunkChild,
&x_root, &y_root, &x, &y, &JunkMask);
 
/* if we haven't recieved the enter notify yet, wait */
-   if (ActiveMenu && !ActiveMenu->entered)
+   if (!ActiveMenu->entered)
continue;
 
if (XFindContext(dpy, ActiveMenu->w, ScreenContext, &context_data) == 0)
-- 
1.7.4.4


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH twm 2/3] Add sanity checking to avoid a possible NULL dereference

2011-05-11 Thread Jeremy Huddleston

menus.c:934:26: warning: Access to field 'fore' results in a dereference of a 
null pointer (loaded from variable 'cur')
cur->hi_back = cur->fore = f3.pixel;
   ~~~  ^

Found-by: clang static analyzer
Signed-off-by: Jeremy Huddleston 
---
 src/menus.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/menus.c b/src/menus.c
index 34a3c8e..395426f 100644
--- a/src/menus.c
+++ b/src/menus.c
@@ -920,7 +920,7 @@ MakeMenu(MenuRoot *mr)
b3.flags = DoRed | DoGreen | DoBlue;
 
num -= 1;
-   for (i = 0, cur = start->next; i < num; i++, cur = cur->next)
+   for (i = 0, cur = start->next; i < num && cur; i++, cur = cur->next)
{
f3.red += fred;
f3.green += fgreen;
-- 
1.7.4.4


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH twm 3/3] Add extra sanity checking to avoid possible NULL dereferences

2011-05-11 Thread Jeremy Huddleston

menus.c:569:10: warning: Access to field 'func' results in a dereference of a 
null pointer (loaded from variable 'ActiveItem')
if (ActiveItem->func != F_TITLE && !ActiveItem->state)
^~

Found-by: clang static analyzer
Signed-off-by: Jeremy Huddleston 
---
 src/menus.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/menus.c b/src/menus.c
index 395426f..1bf32bf 100644
--- a/src/menus.c
+++ b/src/menus.c
@@ -569,7 +569,7 @@ UpdateMenu()
if (!done)
{
ActiveItem = mi;
-   if (ActiveItem->func != F_TITLE && !ActiveItem->state)
+   if (ActiveItem && ActiveItem->func != F_TITLE && !ActiveItem->state)
{
ActiveItem->state = 1;
PaintEntry(ActiveMenu, ActiveItem, False);
@@ -577,7 +577,7 @@ UpdateMenu()
}
 
/* now check to see if we were over the arrow of a pull right entry */
-   if (ActiveItem->func == F_MENU && 
+   if (ActiveItem && ActiveItem->func == F_MENU && 
((ActiveMenu->width - x) < (ActiveMenu->width >> 1)))
{
MenuRoot *save = ActiveMenu;
-- 
1.7.4.4


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: About kdrive status

2011-05-11 Thread Jeremy Huddleston
FWIW, we're still shipping Xnest, Xephyr, Xvfb, Xfake built from the 
server-1.6-branch because the ones built out of 1.7.x and later no longer work 
(I think something XKB related, but I forget exactly what) and I don't have any 
intention of figuring out a workaround.

I haven't actually looked into building the X.Org server / xfree86 DDX on 
darwin as an alternative, so I can't confirm that they don't work (just that 
it's never been tried by me).  Jamey, do you know who told you that it doesn't 
work, or can you tell me specifically what doesn't work, so I can put it on my 
list of things todo?  It would be nice to at least get that DDX buildable on 
OSX.  Is the problem some dependency on libpciaccess, libdrm, DRI2, etc that 
needs to be better conditionalized?

Thanks,
Jeremy

On May 11, 2011, at 12:56 PM, Jamey Sharp wrote:

> On Wed, May 11, 2011 at 04:50:17PM +0800, Liu Peng wrote:
>> Ajax (Adam Jackson) mentioned that the drivers for kdrive were broken and in
>> every way inferior to the corresponding Xorg drivers.
>> http://lists.freedesktop.org/archives/xorg/2011-April/052821.html
>> 
>> So, what's the status of kdrive, is it obsolete?
> 
> As I recall, there's a consensus that the Xfbdev driver in kdrive is
> completely redundant with the X.Org fbdev driver, but the situation with
> Xephyr and Xfake is less clear.
> 
> With the recent push of the X.Org nested driver, both the Xephyr and
> Xfake kdrive servers have counterparts in X.Org drivers. However, as far
> as I can tell, Xephyr and Xfake should work on Windows and Mac, and I'm
> told that X.Org currently doesn't. Also, the nested driver is not yet as
> functional or as efficient as Xephyr.
> 
> For those reasons, I think Xephyr and Xfake will live a little longer,
> but personally I hope their days are numbered.
> 
> Jamey
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH] randr: check rotated virtual size limits correctly

2011-05-11 Thread Jeremy Huddleston
That looks like it does the right thing... (still... get a Tested-by:)

Reviewed-by: Jeremy Huddleston 

On May 11, 2011, at 7:18 AM, Aaron Plattner wrote:

> Commit d1107918d4626268803b54033a07405122278e7f introduced checks to
> the RandR path that cause RRSetScreenConfig requests to fail if the
> size is too large.  Unfortunately, when RandR 1.1 rotation is enabled
> it compares the rotated screen dimensions to the unrotated limits,
> which causes 90- and 270-degree rotation to fail unless your screen
> happens to be square:
> 
>  X Error of failed request:  BadValue (integer parameter out of range for 
> operation)
>Major opcode of failed request:  153 (RANDR)
>Minor opcode of failed request:  2 (RRSetScreenConfig)
>Value in failed request:  0x780
>Serial number of failed request:  14
>Current serial number in output stream:  14
> 
> Fix this by moving the check above the code that swaps the dimensions
> based on the rotation.
> 
> Signed-off-by: Aaron Plattner 
> ---
> This patch is against server-1.10-branch.
> 
> randr/rrscreen.c |   12 ++--
> 1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/randr/rrscreen.c b/randr/rrscreen.c
> index 1bc1a9e..da6d48d 100644
> --- a/randr/rrscreen.c
> +++ b/randr/rrscreen.c
> @@ -910,12 +910,6 @@ ProcRRSetScreenConfig (ClientPtr client)
>  */
> width = mode->mode.width;
> height = mode->mode.height;
> -if (rotation & (RR_Rotate_90|RR_Rotate_270))
> -{
> - width = mode->mode.height;
> - height = mode->mode.width;
> -}
> -
> if (width < pScrPriv->minWidth || pScrPriv->maxWidth < width) {
>   client->errorValue = width;
>   free(pData);
> @@ -927,6 +921,12 @@ ProcRRSetScreenConfig (ClientPtr client)
>   return BadValue;
> }
> 
> +if (rotation & (RR_Rotate_90|RR_Rotate_270))
> +{
> + width = mode->mode.height;
> + height = mode->mode.width;
> +}
> +
> if (width != pScreen->width || height != pScreen->height)
> {
>   int c;
> -- 
> 1.7.4.1
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PULL updated] XQuartz misc-foo

2011-05-12 Thread Jeremy Huddleston
The following changes since commit 5cb31cd0cbf83fff5f17a475e7b0e45246b19bf3:

  Merge remote-tracking branch 'jturney/remove-opengl-spec-download' 
(2011-04-29 09:59:49 -0700)

are available in the git repository at:

  git://people.freedesktop.org/~jeremyhu/xserver master

Jeremy Huddleston (10):
  XQuartz: Use a rwlock instead of a mutex to protect window_hash in the 
pthread case
  XQuartz: Fix incorrect typedefs with XPLUGIN_VERSION < 4
  XQuartz: Ensure that {CF,NS}_RETURNS{,_NOT}_RETAINED are defined
  XQuartz: prefs_copy_url and prefs_get_copy return retained objects
  XQuartz: Make a copy of args for our crash reporter vsnprintf
  XQuartz: Don't call into CoreFoundation after fork() and before exec()
  configure.ac: XQuartz: Fix support for the deprecated 
--with-launchd-id-prefix
  Fix a typo: laucnd instead of launchd
  XQuartz: stub: Log directly to ASL rather than stdout/stderr
  XQuartz: Add a LOGGING section to our man page

 configure.ac  |3 +-
 hw/xquartz/X11Application.h   |3 +-
 hw/xquartz/applewmExt.h   |6 ++--
 hw/xquartz/mach-startup/bundle-main.c |   29 ---
 hw/xquartz/mach-startup/launchd_fd.c  |   18 +
 hw/xquartz/mach-startup/stub.c|   63 -
 hw/xquartz/man/Xquartz.man|   63 ++--
 hw/xquartz/sanitizedCocoa.h   |   39 -
 hw/xquartz/xpr/xprFrame.c |   20 +-
 manpages.am   |2 +-
 os/log.c  |7 +++-
 11 files changed, 169 insertions(+), 84 deletions(-)


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH] dix: Don't implicitly define verify_internal_event

2011-05-14 Thread Jeremy Huddleston

Fixes regression introduced by 56901998020b6f443cbaa5eb303100d979e81b22

Also includes some other style cleanups.

events.c:2198:24: warning: equality comparison with extraneous parentheses 
[-Wparentheses,Semantic Issue]
else if ((type == MotionNotify))
  ~^~~
events.c:2198:24: note: remove extraneous parentheses around the comparison to 
silence this warning [Semantic Issue]
else if ((type == MotionNotify))
 ~ ^  ~
events.c:2198:24: note: use '=' to turn this equality comparison into an 
assignment [Semantic Issue]
else if ((type == MotionNotify))
   ^~
   =
events.c:2487:5: error: implicit declaration of function 
'verify_internal_event' is invalid in C99 
[-Wimplicit-function-declaration,Semantic Issue]
verify_internal_event(event);
^
events.c:5909:22: warning: declaration shadows a local variable 
[-Wshadow,Semantic Issue]
DeviceIntPtr it = inputInfo.devices;
 ^
events.c:5893:18: note: previous declaration is here
DeviceIntPtr it = inputInfo.devices;
 ^
3 warnings and 1 error generated.

events.c:2836:27: warning: incompatible pointer types passing 'DeviceEvent *' 
(aka 'struct _DeviceEvent *') to parameter of type
  'const InternalEvent *' (aka 'const union _InternalEvent *')
verify_internal_event(ev);
  ^~
../include/inpututils.h:40:56: note: passing argument to parameter 'ev' here
extern void verify_internal_event(const InternalEvent *ev);
   ^
1 warning generated.

Found-by: yuffie tinderbox (-Werror=implicit)
Signed-off-by: Jeremy Huddleston 
---
 dix/events.c |7 ---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/dix/events.c b/dix/events.c
index 14f6f90..b60c299 100644
--- a/dix/events.c
+++ b/dix/events.c
@@ -112,6 +112,7 @@ Equipment Corporation.
 #include 
 #include "windowstr.h"
 #include "inputstr.h"
+#include "inpututils.h"
 #include "scrnintstr.h"
 #include "cursorstr.h"
 
@@ -2195,7 +2196,7 @@ DeliverEventsToWindow(DeviceIntPtr pDev, WindowPtr pWin, 
xEvent
  */
 if (!grab && ActivateImplicitGrab(pDev, client, pWin, pEvents, 
deliveryMask))
 /* grab activated */;
-else if ((type == MotionNotify))
+else if (type == MotionNotify)
 pDev->valuator->motionHintWindow = pWin;
 else if (type == DeviceMotionNotify || type == DeviceButtonPress)
 CheckDeviceGrabAndHintWindow (pWin, type,
@@ -2832,7 +2833,7 @@ CheckMotion(DeviceEvent *ev, DeviceIntPtr pDev)
 WindowPtr prevSpriteWin, newSpriteWin;
 SpritePtr pSprite = pDev->spriteInfo->sprite;
 
-verify_internal_event(ev);
+verify_internal_event((InternalEvent *)ev);
 
 prevSpriteWin = pSprite->win;
 
@@ -5906,7 +5907,7 @@ PickPointer(ClientPtr client)
 
 if (!client->clientPtr)
 {
-DeviceIntPtr it = inputInfo.devices;
+it = inputInfo.devices;
 while (it)
 {
 if (IsMaster(it) && it->spriteInfo->spriteOwner)
-- 
1.7.4.1


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH:xdm 5/5] Only greeter needs XPM CFLAGS & LIBS, not xdm daemon

2011-05-14 Thread Jeremy Huddleston
For all 5:
Reviewed-by: Jeremy Huddleston  Signed-off-by: Alan Coopersmith 
> ---
> configure.ac |2 --
> 1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 38717d0..089329f 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -325,8 +325,6 @@ AC_ARG_ENABLE(xpm-logos,
>   PKG_CHECK_EXISTS(xpm, [USE_XPM="yes"], [USE_XPM="no"]))
> if test "x$USE_XPM" = "xyes" ; then
>   PKG_CHECK_MODULES(XPM, xpm)
> - XDM_CFLAGS="$XDM_CFLAGS $XPM_CFLAGS"
> - XDM_LIBS="$XDM_LIBS $XPM_LIBS"
>   GREETER_CFLAGS="$GREETER_CFLAGS $XPM_CFLAGS"
>   GREETER_LIBS="$GREETER_LIBS $XPM_LIBS"
>   AC_DEFINE([XPM], 1, 
> -- 
> 1.7.3.2
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH updated] input: Don't implicitly define verify_internal_event

2011-05-14 Thread Jeremy Huddleston
I missed the one in mieq.c in the first patch.  This should fully fix it now.

---

>From 2f6a3c12b2d344e6cb38c89423f1227453e9a0b4 Mon Sep 17 00:00:00 2001
From: Jeremy Huddleston 
Date: Sat, 14 May 2011 12:23:44 -0700
Subject: [PATCH] input: Don't implicitly define verify_internal_event

Fixes regression introduced by 56901998020b6f443cbaa5eb303100d979e81b22

mieq.c:159:5: error: implicit declaration of function 'verify_internal_event' 
is invalid in C99 [-Wimplicit-function-declaration,Semantic Issue]
verify_internal_event(e);
^
1 error generated.

Also includes some other warning cleanups in events.c we're there.

events.c:2198:24: warning: equality comparison with extraneous parentheses 
[-Wparentheses,Semantic Issue]
else if ((type == MotionNotify))
  ~^~~
events.c:2198:24: note: remove extraneous parentheses around the comparison to 
silence this warning [Semantic Issue]
else if ((type == MotionNotify))
 ~ ^  ~
events.c:2198:24: note: use '=' to turn this equality comparison into an 
assignment [Semantic Issue]
else if ((type == MotionNotify))
   ^~
   =
events.c:2487:5: error: implicit declaration of function 
'verify_internal_event' is invalid in C99 
[-Wimplicit-function-declaration,Semantic Issue]
verify_internal_event(event);
^
events.c:5909:22: warning: declaration shadows a local variable 
[-Wshadow,Semantic Issue]
DeviceIntPtr it = inputInfo.devices;
 ^
events.c:5893:18: note: previous declaration is here
DeviceIntPtr it = inputInfo.devices;
 ^
3 warnings and 1 error generated.

events.c:2836:27: warning: incompatible pointer types passing 'DeviceEvent *' 
(aka 'struct _DeviceEvent *') to parameter of type
  'const InternalEvent *' (aka 'const union _InternalEvent *')
verify_internal_event(ev);
  ^~
../include/inpututils.h:40:56: note: passing argument to parameter 'ev' here
extern void verify_internal_event(const InternalEvent *ev);
       ^
1 warning generated.

Found-by: yuffie tinderbox (-Werror=implicit)
Signed-off-by: Jeremy Huddleston 
---
 dix/events.c |7 ---
 mi/mieq.c|1 +
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/dix/events.c b/dix/events.c
index 14f6f90..b60c299 100644
--- a/dix/events.c
+++ b/dix/events.c
@@ -112,6 +112,7 @@ Equipment Corporation.
 #include 
 #include "windowstr.h"
 #include "inputstr.h"
+#include "inpututils.h"
 #include "scrnintstr.h"
 #include "cursorstr.h"
 
@@ -2195,7 +2196,7 @@ DeliverEventsToWindow(DeviceIntPtr pDev, WindowPtr pWin, 
xEvent
  */
 if (!grab && ActivateImplicitGrab(pDev, client, pWin, pEvents, 
deliveryMask))
 /* grab activated */;
-else if ((type == MotionNotify))
+else if (type == MotionNotify)
 pDev->valuator->motionHintWindow = pWin;
 else if (type == DeviceMotionNotify || type == DeviceButtonPress)
 CheckDeviceGrabAndHintWindow (pWin, type,
@@ -2832,7 +2833,7 @@ CheckMotion(DeviceEvent *ev, DeviceIntPtr pDev)
 WindowPtr prevSpriteWin, newSpriteWin;
 SpritePtr pSprite = pDev->spriteInfo->sprite;
 
-verify_internal_event(ev);
+verify_internal_event((InternalEvent *)ev);
 
 prevSpriteWin = pSprite->win;
 
@@ -5906,7 +5907,7 @@ PickPointer(ClientPtr client)
 
 if (!client->clientPtr)
 {
-DeviceIntPtr it = inputInfo.devices;
+it = inputInfo.devices;
 while (it)
 {
 if (IsMaster(it) && it->spriteInfo->spriteOwner)
diff --git a/mi/mieq.c b/mi/mieq.c
index 031b11a..fc3738a 100644
--- a/mi/mieq.c
+++ b/mi/mieq.c
@@ -43,6 +43,7 @@ in this Software without prior written authorization from The 
Open Group.
 # include   "windowstr.h"
 # include   "pixmapstr.h"
 # include   "inputstr.h"
+# include   "inpututils.h"
 # include   "mi.h"
 # include   "mipointer.h"
 # include   "scrnintstr.h"
-- 
1.7.4.1


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH] input: Fix format string for verify_internal_event

2011-05-14 Thread Jeremy Huddleston

inpututils.c:577:25: warning: conversion specifies type 'unsigned short' but 
the argument has type 'unsigned char' [-Wformat,Format String Issue]
ErrorF("%02hx ", *data);
^~
%02hhx
1 warning generated.

Signed-off-by: Jeremy Huddleston 
---
 dix/inpututils.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/dix/inpututils.c b/dix/inpututils.c
index aeace6e..49e1758 100644
--- a/dix/inpututils.c
+++ b/dix/inpututils.c
@@ -574,7 +574,7 @@ void verify_internal_event(const InternalEvent *ev)
 
 for (i = 0; i < sizeof(xEvent); i++, data++)
 {
-ErrorF("%02hx ", *data);
+ErrorF("%02hhx ", *data);
 
 if ((i % 8) == 7)
 ErrorF("\n");
-- 
1.7.4.1


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH] input: Fix format string for verify_internal_event

2011-05-16 Thread Jeremy Huddleston

On May 16, 2011, at 01:13, Mark Kettenis wrote:

> On Sat, May 14, 2011 at 04:31:10PM -0700, Jeremy Huddleston wrote:
>> 
>> inpututils.c:577:25: warning: conversion specifies type 'unsigned short' but 
>> the argument has type 'unsigned char' [-Wformat,Format String Issue]
>>ErrorF("%02hx ", *data);
>>^~
>>%02hhx
>> 1 warning generated.
> 
> While the fix isn't wrong, it's odd that clang complains about this.
> Since ErrorF is a varargs function, *data will be promoted to int, so
> while printing it with %02hx is a bit odd, it is perfectly legal C.
> Because of the integer promotion, I would actually simply use %02x
> here, since I think the h and hh length modifiers are a bit obscure.
> Would clang complain about using %02x as well?

Yes, and I believe gcc will complain about it as well if you specify a strict 
enough -Wformat.  The warning is there to ensure that you know what you are 
doing.  Yes, functionally they probably result in the bits output, but 
eliminating warnings helps the important ones stand out more.

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH v2] Try and get overlapping cases fixed.

2011-05-16 Thread Jeremy Huddleston
Is the one div needed for:

bpp / 8
bpp % 8

really universally faster than the two bitwise ops needed for

bpp >> 3
bpp & 0x7

?  I'm sure most modern compilers will know how to optimize that based on the 
target CPU, but I've always tried to avoid doing mults and divs in fast paths 
where possible.

--Jeremy

On May 16, 2011, at 09:08, Cyril Brulebois wrote:

> From: Adam Jackson 
> 
> The memcpy fast path implicitly assumes that the copy walks
> left-to-right.  That's not something memcpy guarantees, and newer glibc
> on some processors will indeed break that assumption.  Since we walk a
> line at a time, check the source and destination against the width of
> the blit to determine whether we can be sloppy enough to allow memcpy.
> (Having done this, we can remove the check for !reverse as well.)
> 
> On an Intel Core i7-2630QM with an NVIDIA GeForce GTX 460M running in
> NoAccel, the broken code and various fixes for -copywinwin{10,100,500}
> gives (edited to fit in 80 columns):
> 
> 1: Disable the fastpath entirely
> 2: Replace memcpy with memmove
> 3: This fix
> 4: The code before this fix
> 
>  12 3 4   Operation
> --   ---   ---   ---   
> 258000   269000 (  1.04)   544000 (  2.11)   552000 (  2.14)   Copy 10x10
> 2130023000 (  1.08)43700 (  2.05)47100 (  2.21)   Copy 100x100
>   960  962 (  1.00) 1990 (  2.09) 1990 (  2.07)   Copy 500x500
> 
> So it's a modest performance hit, but correctness demands it, and it's
> probably worth keeping the 2x speedup from having the fast path in the
> first place.
> 
> Signed-off-by: Adam Jackson 
> 
> v2: Fix limit cases thanks to Soeren Sandmann, and apply a tiny
>optimization by Walter Harms.
> 
> Signed-off-by: Cyril Brulebois 
> ---
> fb/fbblt.c |5 -
> 1 files changed, 4 insertions(+), 1 deletions(-)
> 
> 
> Tested on amd64 on top of xorg-server's server-1.10-branch.
> 
> 
> diff --git a/fb/fbblt.c b/fb/fbblt.c
> index 38271c0..b6e7785 100644
> --- a/fb/fbblt.c
> +++ b/fb/fbblt.c
> @@ -65,6 +65,7 @@ fbBlt (FbBits   *srcLine,
> int   n, nmiddle;
> BooldestInvarient;
> int   startbyte, endbyte;
> +int careful;
> FbDeclareMergeRop ();
> 
> #ifdef FB_24BIT
> @@ -76,7 +77,9 @@ fbBlt (FbBits   *srcLine,
> }
> #endif
> 
> -if (alu == GXcopy && pm == FB_ALLONES && !reverse &&
> +careful = (width * (bpp / 8) > abs(srcLine-dstLine)) || (bpp % 8);
> +
> +if (alu == GXcopy && pm == FB_ALLONES && !careful &&
> !(srcX & 7) && !(dstX & 7) && !(width & 7)) {
> int i;
> CARD8 *src = (CARD8 *) srcLine;
> -- 
> 1.7.5.1
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PULL] XQuartz crash fixes

2011-05-16 Thread Jeremy Huddleston
The following changes since commit 0e7f61d72c4a929319e57c9b5b777e9413c23051:

  doc: use devbook.am for developers documentation (2011-05-14 11:22:29 -0700)

are available in the git repository at:
  git://people.freedesktop.org/~jeremyhu/xserver master

Jeremy Huddleston (3):
  XQuartz: Fix an array-index-out-of-bounds crasher
  XQuartz: Don't call mieqEnqueue during server shutdown
  XQuartz: RandR: Avoid over-releasing if we are unable to determine the 
current display mode.

 dix/main.c|   25 -
 hw/xquartz/console_redirect.c |2 +-
 hw/xquartz/pbproxy/app-main.m |6 +++---
 hw/xquartz/pbproxy/main.m |   16 
 hw/xquartz/quartzRandR.c  |   19 +++
 mi/mieq.c |   16 
 6 files changed, 47 insertions(+), 37 deletions(-)

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH:libXfont] Fix memory leak in allocation failure path of BitmapOpenScalable()

2011-05-17 Thread Jeremy Huddleston
Reviewed-by: Jeremy Huddleston 

On May 16, 2011, at 18:33, Alan Coopersmith wrote:

> Go ahead and fill in the font->info pointers so that bitmapUnloadScalable()
> will free the bits that were allocated, even if some were not.
> 
> Error: Memory leak (CWE 401)
>   Memory leak of pointer  allocated with ComputeScaledProperties(...)
>at line 1629 of 
> /export/alanc/X.Org/git/lib/libXfont/src/bitmap/bitscale.c in function 
> 'BitmapOpenScalable'.
>  pointer allocated at line 1616 with ComputeScaledProperties(...).
>   leaks when props != 0 at line 1623.
> 
> [ This bug was found by the Parfait 0.3.7 bug checking tool.
>  For more information see http://labs.oracle.com/projects/parfait/ ]
> 
> Signed-off-by: Alan Coopersmith 
> ---
> src/bitmap/bitscale.c |   11 ---
> 1 files changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/src/bitmap/bitscale.c b/src/bitmap/bitscale.c
> index cf16bff..50818c6 100644
> --- a/src/bitmap/bitscale.c
> +++ b/src/bitmap/bitscale.c
> @@ -1620,19 +1620,16 @@ BitmapOpenScalable (FontPathElementPtr fpe,
> if (!sourceFont->refcnt)
>   FontFileCloseFont((FontPathElementPtr) 0, sourceFont);
> 
> +font->info.props = props;
> +font->info.nprops = propCount;
> +font->info.isStringProp = isStringProp;
> +
> if (propCount && (!props || !isStringProp))
> {
> - font->info.nprops = 0;
> - font->info.props = (FontPropPtr)0;
> - font->info.isStringProp = (char *)0;
>   bitmapUnloadScalable(font);
>   return AllocError;
> }
> 
> -font->info.props = props;
> -font->info.nprops = propCount;
> -font->info.isStringProp = isStringProp;
> -
> *pFont = font;
> return Successful;
> }
> -- 
> 1.7.3.2
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH evdev 3/4] Add a property to toggle fnmode on Apple keyboards

2011-05-18 Thread Jeremy Huddleston
Hey Peter,

I'm guessing that proxying this property in the driver will make it easier for 
generalized preference panes to be created based on Xinput.  I seem to recall 
filing a bug somewhere a couple years ago about the lack of GUI surrounding 
this option in Gnome's preferences, so this gets a big:

Acked-by: Jeremy Huddleston 
Reviewed-by: Jeremy Huddleston 

--Jeremy

On May 17, 2011, at 22:00, Peter Hutterer wrote:

> On Apple keyboards, the multimedia function keys are overlaid with the F
> keys. F1 is also BrightnessDown, F10 is Mute, etc. The kernel provides a
> tweak to enable/disable this.
> 
> /sys/module/hid_apple/parameters/fnmode
>0 .. keyboard sends Fx keys
>1 .. keyboard sends multimedia keys
> 
> Signed-off-by: Peter Hutterer 
> ---
> include/evdev-properties.h |3 +
> src/Makefile.am|3 +-
> src/apple.c|  168 
> src/evdev.c|1 +
> src/evdev.h|1 +
> 5 files changed, 175 insertions(+), 1 deletions(-)
> create mode 100644 src/apple.c
> 
> diff --git a/include/evdev-properties.h b/include/evdev-properties.h
> index 16f2af7..8887cd1 100644
> --- a/include/evdev-properties.h
> +++ b/include/evdev-properties.h
> @@ -75,4 +75,7 @@
> /* CARD32 */
> #define EVDEV_PROP_THIRDBUTTON_THRESHOLD "Evdev Third Button Emulation 
> Threshold"
> 
> +/* BOOL, 1 value, true → send function keys, false → send multimedia keys */
> +#define EVDEV_PROP_APPLE_FNMODE "Evdev Apple Function Keys"
> +
> #endif
> diff --git a/src/Makefile.am b/src/Makefile.am
> index d1efe53..b3a5671 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -37,5 +37,6 @@ AM_CPPFLAGS =-I$(top_srcdir)/include
>emuMB.c \
>emuThird.c \
>emuWheel.c \
> -   draglock.c
> +   draglock.c \
> +   apple.c
> 
> diff --git a/src/apple.c b/src/apple.c
> new file mode 100644
> index 000..1a82e2a
> --- /dev/null
> +++ b/src/apple.c
> @@ -0,0 +1,168 @@
> +/*
> + * Copyright © 2011 Red Hat, Inc.
> + *
> + * Permission to use, copy, modify, distribute, and sell this software
> + * and its documentation for any purpose is hereby granted without
> + * fee, provided that the above copyright notice appear in all copies
> + * and that both that copyright notice and this permission notice
> + * appear in supporting documentation, and that the name of Red Hat
> + * not be used in advertising or publicity pertaining to distribution
> + * of the software without specific, written prior permission.  Red
> + * Hat makes no representations about the suitability of this software
> + * for any purpose.  It is provided "as is" without express or implied
> + * warranty.
> + *
> + * THE AUTHORS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN
> + * NO EVENT SHALL THE AUTHORS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
> + * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
> + * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
> + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + *
> + * Authors:
> + *   Peter Hutterer (peter.hutte...@redhat.com)
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* Apple-specific controls.
> + *
> + * On Apple keyboards, the multimedia function keys are overlaid with the F
> + * keys. F1 is also BrightnessDown, F10 is Mute, etc. The kernel provides a
> + * tweak to enable/disable this.
> + *
> + * /sys/module/hid_apple/parameters/fnmode
> + * 0 .. keyboard sends Fx keys
> + * 1 .. keyboard sends multimedia keys
> + */
> +
> +#define FNMODE_PATH "/sys/module/hid_apple/parameters/fnmode"
> +#define APPLE_VENDOR 0x5ac
> +#define APPLE_KEYBOARD 0x220
> +
> +static Atom prop_fnmode;
> +static Bool fnmode_readonly; /* set if we can only read fnmode */
> +
> +
> +/**
> + * @retval 0 fnmode is set to function keys
> + * @retval 1 fnmode is set to multimedia keys
> + * @retval -1 Error, see errno
> + */
> +static int
> +get_fnmode(void)
> +{
> +int fd;
> +char retvalue;
> +
> +fd = open(FNMODE_PATH, O_RDWR);
> +if (fd < 0)
> +{
>

Re: [PATCH] makekeys: Fix build/target word size mismatch when cross-compiling

2011-05-20 Thread Jeremy Huddleston
cross-compilation is a PITA.

Reviewed-by: Jeremy Huddleston 


On May 20, 2011, at 08:05, Derek Buitenhuis wrote:

> Since makekeys is built using build environment's gcc and
> run natively, we have to make sure that the size of the
> Signature type is the same on both the native build environment
> and the host, otherwise we get mismatches upon running X,
> and some LSB test failures (xts5).
> 
> Have configure check the size of the host's unsigned long
> and typedef Signature in makekeys.c accordingly.
> ---
> configure.ac|3 +++
> src/util/makekeys.c |   13 ++---
> 2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index a39ab8d..16108a5 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -192,6 +192,9 @@ else
> fi
> AC_MSG_RESULT($HAVE_LOADABLE_MODULES)
> 
> +#Check the size of unsigned long for use in makekeys
> +AC_CHECK_SIZEOF([unsigned long])
> +
> AC_MSG_CHECKING([if loadable i18n module support should be enabled])
> AC_ARG_ENABLE(loadable-i18n,
>   AS_HELP_STRING([--enable-loadable-i18n],
> diff --git a/src/util/makekeys.c b/src/util/makekeys.c
> index 8f88beb..6022f3b 100644
> --- a/src/util/makekeys.c
> +++ b/src/util/makekeys.c
> @@ -31,10 +31,17 @@ from The Open Group.
> #include 
> #include 
> #include 
> +#include 
> #include 
> #include 
> 
> -typedef unsigned long Signature;
> +#include "../config.h"
> +
> +#if SIZEOF_UNSIGNED_LONG == 4
> +typedef uint32_t Signature;
> +#else
> +typedef uint64_t Signature;
> +#endif
> 
> #define KTNUM 4000
> 
> @@ -212,8 +219,8 @@ next1:;
> offsets[j] = k;
> indexes[i] = k;
> val = info[i].val;
> -printf("0x%.2lx, 0x%.2lx, 0x%.2lx, 0x%.2lx, 0x%.2lx, 0x%.2lx, ",
> -   (sig >> 8) & 0xff, sig & 0xff,
> +printf("0x%.2jx, 0x%.2jx, 0x%.2lx, 0x%.2lx, 0x%.2lx, 0x%.2lx, ",
> +   (uintmax_t)((sig >> 8) & 0xff), (uintmax_t)(sig & 0xff),
>(val >> 24) & 0xff, (val >> 16) & 0xff,
>(val >> 8) & 0xff, val & 0xff);
> for (name = info[i].name, k += 7; (c = *name++); k++)
> -- 
> 1.7.4.1
> 
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: Coping with -Wunused-but-set-variable, first round

2011-05-20 Thread Jeremy Huddleston
On the whole, this gets my ACK as this will clear up a bit of the clang static 
analysis noise, but some of the changes feel suspicious to me.  It feels like 
some of the dead code was there for a reason, and maybe the fix is to *use* the 
value properly.  In other cases where I'm more familiar with the code, it looks 
clear to me that values once were needed and changes in "the right way" meant 
we no longer passed those values along.

Did you do a 'git blame' to figure out when the dead code was added to get some 
context and make sure that the once-needed values are really no longer 
required?  Obviously things "work" now, but I'd hate to come back in 2 years to 
find out that we acted the wrong way on a particular warning.


On May 20, 2011, at 08:34, Cyril Brulebois wrote:

> Hi,
> 
> gcc 4.6 has -Wunused-but-set-variable[1]:
> ,
> | New -Wunused-but-set-variable and -Wunused-but-set-parameter warnings
> | were added for C, C++, Objective-C and Objective-C++. These warnings
> | diagnose variables respective parameters which are only set in the
> | code and never otherwise used. Usually such variables are useless and
> | often even the value assigned to them is computed needlessly,
> | sometimes expensively. The -Wunused-but-set-variable warning is
> | enabled by default by -Wall flag and -Wunused-but-set-parameter by
> | -Wall -Wextra flags.
> `
> 
> 1. http://gcc.gnu.org/gcc-4.6/changes.html
> 
> We have a lot of hits, but with the upcoming patchset (30 patches…) we
> go down from 126 to 91; remaining parts include fb*, shrotpack*.h,
> render2swap.c stuff, which I was a bit reluctant to patch without
> thinking of it a bit. So the patchset should only have “easy to
> ACK/NACK” items.
> 
> It's also available in my repository:
>  git://anongit.freedesktop.org/~kibi/xserver unused-but-set-variable
> 
> Mraw,
> KiBi.
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: Coping with -Wunused-but-set-variable, first round

2011-05-20 Thread Jeremy Huddleston
I would include the warning message as well.  gcc might not provide the best 
warning, so if you're feeling so inclined, you can check if there is a 
corresponding report in the clang static analysis reports:

http://people.freedesktop.org/~jeremyhu/analyzer

or just check the build log directly on tinderbox:

http://tinderbox.x.org



On May 20, 2011, at 08:37, Cyril Brulebois wrote:

> Cyril Brulebois  (20/05/2011):
>> We have a lot of hits, but with the upcoming patchset (30 patches…) we
>> go down from 126 to 91; remaining parts include fb*, shrotpack*.h,
>> render2swap.c stuff, which I was a bit reluctant to patch without
>> thinking of it a bit. So the patchset should only have “easy to
>> ACK/NACK” items.
>> 
>> It's also available in my repository:
>>  git://anongit.freedesktop.org/~kibi/xserver unused-but-set-variable
> 
> And I forgot to ask this question: should I include some boilerplate
> in each commit message? (Including the actual warning message wouldn't
> bring much AFAICT, and would mean more work.)
> 
> If so, what? Just “Detected by -Wunused-but-set-variable, introduced
> in gcc 4.6” or something similar?
> 
> Mraw,
> KiBi.
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH] "privates.h", line 198: warning: void function cannot return value

2011-05-20 Thread Jeremy Huddleston
Reviewed-by: Jeremy Huddleston 


On May 20, 2011, at 9:17 PM, Alan Coopersmith wrote:

> Providing an argument to return in a function with void return type
> is not allowed by the C standard, and makes the Sun compilers unhappy.
> 
> Signed-off-by: Alan Coopersmith 
> ---
> include/privates.h |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/privates.h b/include/privates.h
> index 7ef2cb7..2b0040c 100644
> --- a/include/privates.h
> +++ b/include/privates.h
> @@ -195,7 +195,7 @@ dixGetScreenPrivate(PrivatePtr *privates, const 
> DevScreenPrivateKey key, ScreenP
> static inline void
> dixSetScreenPrivate(PrivatePtr *privates, const DevScreenPrivateKey key, 
> ScreenPtr pScreen, pointer val)
> {
> -return dixSetPrivate(privates, _dixGetScreenPrivateKey(key, pScreen), 
> val);
> +dixSetPrivate(privates, _dixGetScreenPrivateKey(key, pScreen), val);
> }
> 
> static inline pointer
> -- 
> 1.7.3.2
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH] fb: Fix memcpy abuse

2011-05-21 Thread Jeremy Huddleston

On May 21, 2011, at 05:03, Siarhei Siamashka wrote:

> On Sat, Apr 30, 2011 at 3:39 PM, Soeren Sandmann  wrote:
>> 
>> 
>> If I remember correctly, the main objection I had to the overlapped blt
>> patch was that it inlined the C fallback into all the SIMD versions
>> instead of just calling down through the delegate.
> 
> That's quite easy to fix. Too bad that this problem was brought up on
> xorg-devel mailing list a few weeks too late for overlapped pixman_blt
> to be introduced in pixman 0.22.0. My main concern earlier was about
> how to use the newly added pixman features in xserver and provide a
> smooth upgrade path without breaking anything. But now I understand
> that it just requires adding this feature to pixman, then wait till
> the next stable pixman version 0.24.0 goes out, then add the needed
> changes to xserver to use this feature and bump the required pixman
> version to 0.24.0 at the same time. And finally the users will be able
> to enjoy faster non-hardware accelerated scrolling after the next
> stable xserver version gets released and adopted by linux distros.
> Right?

Yeah, that's about right.  Get it into pixman-0.23.x.  Once we actually have a 
0.23.x release of pixman, we can add that functionality to xserver master with 
appropriate dependency-foo added to configure.ac.  Looking at past release 
cycles, my guess is that 0.24 will be out around the same time as 
xorg-server-1.11, so I don't think there will be any objections to such a 
dependency... Soren, can you please confirm my assumptions here =)

> Sure. Just these other things are much less commonly used and do not
> require any urgent attention. It's more like a code
> refactoring/cleanup work but not something that is clearly beneficial
> for the end users.

Refactoring and code cleanup is almost always beneficial for the end users, 
albeit indirectly.  It helps us more easily spot bugs and more easily maintain 
the codebase.

--Jeremy

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH resend xrandr] find_mode: Search for the mode closes to the specified rate

2011-05-21 Thread Jeremy Huddleston
ping

Begin forwarded message:

> From: Jeremy Huddleston 
> Date: May 9, 2011 9:26:34 AM PDT
> To: Keith Packard 
> Cc: "X.Org Devel List" 
> Subject: [PATCH xrandr] find_mode: Search for the mode closes to the 
> specified rate
> 
> Hey Keith,
> 
> clang found an issue in xrandr that I'd like you to review the fix for.  I'm 
> fairly certain this is what you intended.
> 
> Thanks,
> Jeremy
> 
> For reference:
> http://people.freedesktop.org/~jeremyhu/analyzer/yuffie/20110509-/xrandr/report-gZOzCf.html#EndPath
> http://cgit.freedesktop.org/xorg/app/xrandr/commit/?id=d9aeb4a7544ad4a33f6f54bc46bff5cdf231a986
> 
> ---
> 
> From 9f1eb72eea2ba5375be9a2db1e193cda8a50a841 Mon Sep 17 00:00:00 2001
> From: Jeremy Huddleston 
> Date: Mon, 9 May 2011 09:21:29 -0700
> Subject: [PATCH] find_mode: Search for the mode closes to the specified rate
> 
> This was the intention of d9aeb4a7544ad4a33f6f54bc46bff5cdf231a986, but
> find_mode was still picking the first string match rather than the
> match with the closest refresh rate.
> 
> xrandr.c:740:3: warning: Value stored to 'bestDist' is never read
>    bestDist = dist;
>^  
> 
> Found-by: clang static analyzer
> Signed-off-by: Jeremy Huddleston 
> ---
> xrandr.c |1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/xrandr.c b/xrandr.c
> index 564d4da..10d034d 100644
> --- a/xrandr.c
> +++ b/xrandr.c
> @@ -740,7 +740,6 @@ find_mode (name_t *name, double refresh)
>   bestDist = dist;
>   best = mode;
>   }
> - break;
>   }
> }
> return best;
> -- 
> 1.7.4.4
> 
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PULL update] XQaurtz crash fixes and misc

2011-05-22 Thread Jeremy Huddleston
The following changes since commit 0e7f61d72c4a929319e57c9b5b777e9413c23051:

  doc: use devbook.am for developers documentation (2011-05-14 11:22:29 -0700)

are available in the git repository at:
  git://people.freedesktop.org/~jeremyhu/xserver master

Jeremy Huddleston (8):
  XQuartz: Fix an array-index-out-of-bounds crasher
  XQuartz: Don't call mieqEnqueue during server shutdown
  XQuartz: RandR: Avoid over-releasing if we are unable to determine the 
current display mode.
  XQuartz: Update DEBUG_LOG to report to ASL
  XQuartz: Silence clang warnings about shadow declarations
  XQuartz: Mark functions _X_NORETURN
  XQuartz: RandR: Don't crash if X11 is launched while there are no 
attached displays
  XQuartz: Don't crash if CG increases our display resolution

 dix/main.c|   25 +++--
 hw/xquartz/X11Application.m   |6 +-
 hw/xquartz/console_redirect.c |2 +-
 hw/xquartz/darwin.c   |   33 -
 hw/xquartz/darwin.h   |   15 +--
 hw/xquartz/mach-startup/bundle-main.c |5 +-
 hw/xquartz/mach-startup/stub.c|1 +
 hw/xquartz/pbproxy/app-main.m |6 +-
 hw/xquartz/pbproxy/main.m |   16 ++--
 hw/xquartz/quartz.c   |   15 ++-
 hw/xquartz/quartzRandR.c  |  219 ++---
 hw/xquartz/quartzStartup.c|1 +
 hw/xquartz/xpr/xprScreen.c|3 +-
 mi/mieq.c |   16 ++--
 14 files changed, 205 insertions(+), 158 deletions(-)

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Xlib voodoo XWidthOfScreen, etc

2011-05-23 Thread Jeremy Huddleston
I'm having some trouble understanding some of Xlib...

After changing resolution with RandR, all the following seem to report the new 
resolution properly:

$ xdpyinfo
$ xwininfo -root
$ xrandr
XineramaQueryScreens() called within my WM

however, XWidthOfScreen() and XHeightOfScreen() called from within my WM still 
report the old resolution.

I was under the impression that the internals of Display and Screen were 
updated "automagically" within Xlib.  Do I need to do something to force the 
internals to update?

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: Xlib voodoo XWidthOfScreen, etc

2011-05-23 Thread Jeremy Huddleston
Yeah, ok.  You know what happens when I assume... ;)

Looks like I need to do some XRRUpdateConfiguration magic in the WM.  Thanks.


On May 23, 2011, at 10:04, Jamey Sharp wrote:

> On Mon, May 23, 2011 at 12:30:03AM -0700, Jeremy Huddleston wrote:
>> I'm having some trouble understanding some of Xlib...
>> 
>> After changing resolution with RandR, all the following seem to report the 
>> new resolution properly:
>> 
>> $ xdpyinfo
>> $ xwininfo -root
>> $ xrandr
>> XineramaQueryScreens() called within my WM
>> 
>> however, XWidthOfScreen() and XHeightOfScreen() called from within my
>> WM still report the old resolution.
>> 
>> I was under the impression that the internals of Display and Screen
>> were updated "automagically" within Xlib.  Do I need to do something
>> to force the internals to update?
> 
> The connection setup data, which includes per-screen widths and heights,
> is sent only when the connection is set up. Applications that need to
> know about changes to those values need to get RandR events.
> 
> There could be magic in libXrandr to fiddle with the contents of the
> Display, but I'd be surprised... Especially since you'd still need to
> have called into that library to get the magic hooked up.
> 
> Jamey

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH:x11proto] Add comments to Xfuncproto.h noting required xproto versions for each macro

2011-05-23 Thread Jeremy Huddleston
Helpful.

Reviewed-by: Jeremy Huddleston 

On May 23, 2011, at 4:04 PM, Alan Coopersmith wrote:

> Saves time trawling git logs to determine what to put in the call to
> PKG_CHECK_MODULES in configure.ac when you start using one of these.
> 
> Signed-off-by: Alan Coopersmith 
> ---
> Xfuncproto.h.in |   11 +++
> 1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/Xfuncproto.h.in b/Xfuncproto.h.in
> index 39ca3b2..0d7b8b7 100644
> --- a/Xfuncproto.h.in
> +++ b/Xfuncproto.h.in
> @@ -75,12 +75,14 @@ in this Software without prior written authorization from 
> The Open Group.
> #endif
> #endif /* _XFUNCPROTOBEGIN */
> 
> +/* Added in X11R6.9, so available in any version of modular xproto */
> #if defined(__GNUC__) && (__GNUC__ >= 4)
> # define _X_SENTINEL(x) __attribute__ ((__sentinel__(x)))
> #else
> # define _X_SENTINEL(x)
> #endif /* GNUC >= 4 */
> 
> +/* Added in X11R6.9, so available in any version of modular xproto */
> #if defined(__GNUC__) && (__GNUC__ >= 4) && !defined(__CYGWIN__)
> # define _X_EXPORT  __attribute__((visibility("default")))
> # define _X_HIDDEN  __attribute__((visibility("hidden")))
> @@ -95,6 +97,7 @@ in this Software without prior written authorization from 
> The Open Group.
> # define _X_INTERNAL
> #endif /* GNUC >= 4 */
> 
> +/* requires xproto >= 7.0.9 */
> #if defined(__GNUC__) && ((__GNUC__ * 100 + __GNUC_MINOR__) >= 303)
> # define _X_LIKELY(x)   __builtin_expect(!!(x), 1)
> # define _X_UNLIKELY(x) __builtin_expect(!!(x), 0)
> @@ -103,12 +106,14 @@ in this Software without prior written authorization 
> from The Open Group.
> # define _X_UNLIKELY(x) (x)
> #endif
> 
> +/* Added in X11R6.9, so available in any version of modular xproto */
> #if defined(__GNUC__) && ((__GNUC__ * 100 + __GNUC_MINOR__) >= 301)
> # define _X_DEPRECATED  __attribute__((deprecated))
> #else /* not gcc >= 3.1 */
> # define _X_DEPRECATED
> #endif
> 
> +/* requires xproto >= 7.0.17 */
> #if (defined(__GNUC__) && ((__GNUC__ * 100 + __GNUC_MINOR__) >= 205)) \
>   || (defined(__SUNPRO_C) && (__SUNPRO_C >= 0x590))
> # define _X_NORETURN __attribute((noreturn))
> @@ -116,18 +121,21 @@ in this Software without prior written authorization 
> from The Open Group.
> # define _X_NORETURN
> #endif /* GNUC  */
> 
> +/* Added in X11R6.9, so available in any version of modular xproto */
> #if defined(__GNUC__) && ((__GNUC__ * 100 + __GNUC_MINOR__) >= 203)
> # define _X_ATTRIBUTE_PRINTF(x,y) __attribute__((__format__(__printf__,x,y)))
> #else /* not gcc >= 2.3 */
> # define _X_ATTRIBUTE_PRINTF(x,y)
> #endif
> 
> +/* requires xproto >= 7.0.22 */
> #if defined(__GNUC__) &&  ((__GNUC__ * 100 + __GNUC_MINOR__) >= 303)
> #define _X_NONNULL(args...)  __attribute__((nonnull(args)))
> #else
> #define _X_NONNULL(...)  /* */
> #endif
> 
> +/* requires xproto >= 7.0.22 */
> #if defined(__GNUC__) &&  ((__GNUC__ * 100 + __GNUC_MINOR__) >= 205)
> #define _X_UNUSED  __attribute__((__unused__))
> #else
> @@ -135,6 +143,8 @@ in this Software without prior written authorization from 
> The Open Group.
> #endif
> 
> /* C99 keyword "inline" or equivalent extensions in pre-C99 compilers */
> +/* requires xproto >= 7.0.9
> +   (introduced in 7.0.8 but didn't support all compilers until 7.0.9) */
> #if defined(inline) /* assume autoconf set it correctly */ || \
>(defined(__STDC_VERSION__) && (__STDC_VERSION__ - 0 >= 199901L)) /* C99 */ 
> || \
>(defined(__SUNPRO_C) && (__SUNPRO_C >= 0x550))
> @@ -146,6 +156,7 @@ in this Software without prior written authorization from 
> The Open Group.
> #endif
> 
> /* C99 keyword "restrict" or equivalent extensions in pre-C99 compilers */
> +/* requires xproto >= 7.0.21 */
> #ifndef _X_RESTRICT_KYWD
> # if defined(restrict) /* assume autoconf set it correctly */ || \
>(defined(__STDC_VERSION__) && (__STDC_VERSION__ - 0 >= 199901L)) /* C99 */
> -- 
> 1.7.3.2
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


GPLv3 (Re: [ANNOUNCE] xcb-util 0.3.8)

2011-05-24 Thread Jeremy Huddleston
This is the first released tarball from X.org and XCB that requires 
autoconf-2.62 to autoconf.

What is needed from 2.62 that isn't in 2.61?  2.62 is released under GPLv3 
making it very difficult for users to rebuild the configure script.

On Apr 25, 2011, at 09:53, Arnaud Fontaine wrote:

> xcb-util 0.3.8 is now available.
> 
> git tag 0.3.8
> 
> * Changelog
> Alan Coopersmith (2):
>  Fix typos in various header comments
>  Rename XCB_EVENT_ERROR_SUCESS to XCB_EVENT_ERROR_SUCCESS
> 
> Arnaud Fontaine (8):
>  Add AM_MAINTAINER_MODE for vendors
>  Split up atom, aux event into their own repository
>  Create a single shared library and header file and reset SONAME
>  Remove deprecated atoms.
>  Remove synchronous xcb_atom_get()
>  Remove useless xcb_atom_get_fast*() and xcb_atom_get_name().
>  Bump version to 0.3.8
>  Release 0.3.8
> 
> Dirk Wallenstein (1):
>  Use an absolute URL for the m4 submodule
> 
> Gaetan Nadon (5):
>  config: use the default xorg .gitignore file
>  config: generate ChangeLog and INSTALL
>  Fix distcheck due to xcb_atom.h
>  Remove the Doxyfile which is unused, out dated and wrong.
>  pkg-config files: remove LIBS which is unrequired and undesirable
> 
> Jamey Sharp (2):
>  Delete redundant core-protocol error codes.
>  Delete callback-based APIs for events, properties, and replies.
> 
> Jon TURNEY (2):
>  Update autogen.sh to one that does builddir != srcdir
>  Link with -no-undefined
> 
> Peter Harris (1):
>  Deprecate namespaceless pre-defined atoms
> 
> * Download
> http://xcb.freedesktop.org/dist/xcb-util-0.3.8.tar.gz
> md5: 08ae7994646bbd8d741b954d40a0572a
> sha1: 3ee219b79a70c55841a26f8bde2edf923ab80964
> 
> http://xcb.freedesktop.org/dist/xcb-util-0.3.8.tar.bz2
> md5: 8ce019c4bbf20dce246b98f177cfccff
> sha1: 71093ad81feb21758a2446cf3297bebaf03af228
> 
> Cheers,
> -- 
> Arnaud Fontaine
> ___
> xorg-announce mailing list
> xorg-annou...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xorg-announce

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH:mkfontscale] Add _X_ATTRIBUTE_PRINTF to functions taking printf format arguments

2011-05-24 Thread Jeremy Huddleston
Reviewed-by: Jeremy Huddleston 

On May 23, 2011, at 18:34, Alan Coopersmith wrote:

> Signed-off-by: Alan Coopersmith 
> ---
> list.h |8 +---
> 1 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/list.h b/list.h
> index 463933d..686fd9b 100644
> --- a/list.h
> +++ b/list.h
> @@ -23,7 +23,9 @@
> #ifndef _MKS_LIST_H_
> #define _MKS_LIST_H_ 1
> 
> -char *dsprintf(char *f, ...);
> +#include  /* for _X_ATTRIBUTE_PRINTF */
> +
> +char *dsprintf(char *f, ...) _X_ATTRIBUTE_PRINTF(1,2);
> 
> typedef struct _List {
> char *value;
> @@ -33,8 +35,8 @@ typedef struct _List {
> int listMember(char *elt, ListPtr list);
> ListPtr listCons(char *car, ListPtr cdr);
> ListPtr listAdjoin(char *car, ListPtr cdr);
> -ListPtr listConsF(ListPtr cdr, char *f, ...);
> -ListPtr listAdjoinF(ListPtr cdr, char *f, ...);
> +ListPtr listConsF(ListPtr cdr, char *f, ...) _X_ATTRIBUTE_PRINTF(2,3);
> +ListPtr listAdjoinF(ListPtr cdr, char *f, ...) _X_ATTRIBUTE_PRINTF(2,3);
> int listLength(ListPtr list);
> ListPtr appendList(ListPtr first, ListPtr second);
> ListPtr makeList(char **a, int n, ListPtr old, int begin);
> -- 
> 1.7.3.2
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 03/14] dix: Avoid implicit declaration of 'verify_internal_event'.

2011-05-24 Thread Jeremy Huddleston
Peter already has this one in his tree.

On May 24, 2011, at 09:41, Cyril Brulebois wrote:

> Those warnings go away accordingly:
> |   CC events.lo
> | events.c: In function 'DeliverDeviceEvents':
> | events.c:2485:5: warning: implicit declaration of function 
> 'verify_internal_event' [-Wimplicit-function-declaration]
> | events.c:2485:5: warning: nested extern declaration of 
> 'verify_internal_event' [-Wnested-externs]
> 
> The following warning remains, though:
> |   CC events.lo
> | events.c: In function 'CheckMotion':
> | events.c:2835:5: warning: passing argument 1 of 'verify_internal_event' 
> from incompatible pointer type [enabled by default]
> | ../include/inpututils.h:40:13: note: expected 'const union InternalEvent *' 
> but argument is of type 'struct DeviceEvent *'
> 
> Signed-off-by: Cyril Brulebois 
> ---
> dix/events.c |2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/dix/events.c b/dix/events.c
> index 126d825..071802b 100644
> --- a/dix/events.c
> +++ b/dix/events.c
> @@ -152,6 +152,8 @@ typedef const char *string;
> #include "enterleave.h"
> #include "eventconvert.h"
> 
> +#include "inpututils.h"
> +
> /* Extension events type numbering starts at EXTENSION_EVENT_BASE.  */
> #define NoSuchEvent 0x8000/* so doesn't match NoEventMask */
> #define StructureAndSubMask ( StructureNotifyMask | SubstructureNotifyMask )
> -- 
> 1.7.5.1
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 04/14] mi: Avoid implicit declaration of 'verify_internal_event'.

2011-05-24 Thread Jeremy Huddleston
This one is also already in Peter's tree.

On May 24, 2011, at 09:41, Cyril Brulebois wrote:

> Those warnings go away accordingly:
> |   CC mieq.lo
> | mieq.c: In function 'mieqEnqueue':
> | mieq.c:159:5: warning: implicit declaration of function 
> 'verify_internal_event' [-Wimplicit-function-declaration]
> | mieq.c:159:5: warning: nested extern declaration of 'verify_internal_event' 
> [-Wnested-externs]
> 
> Signed-off-by: Cyril Brulebois 
> ---
> mi/mieq.c |1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/mi/mieq.c b/mi/mieq.c
> index 236ffcc..b71bec4 100644
> --- a/mi/mieq.c
> +++ b/mi/mieq.c
> @@ -52,6 +52,7 @@ in this Software without prior written authorization from 
> The Open Group.
> # include   "extinit.h"
> # include   "exglobals.h"
> # include   "eventstr.h"
> +# include   "inpututils.h"
> 
> #ifdef DPMSExtension
> # include "dpmsproc.h"
> -- 
> 1.7.5.1
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH 01/14] Xext: Get rid of useless checks.

2011-05-24 Thread Jeremy Huddleston
1,2:
   Ditto Alan's Response

3,4:
   Duplicates

5,6:
   Reviewed-by: Jeremy Huddleston 

7,8,9:
   Disgusted-by: Jeremy Huddleston 
   Reviewed-by: Jeremy Huddleston 

10:
   Reviewed-by: Jeremy Huddleston 

11:
   I'd rather remove them than mark them unused

12:
   Reviewed-by: Jeremy Huddleston 

13,14:
   Why not just remove the dead code or move the declaration into the #if 0?



On May 24, 2011, at 09:41, Cyril Brulebois wrote:

> As noted in a comment, that can't actually happen.
> 
> Those warnings go away accordingly:
> |   CC xvmc.lo
> | xvmc.c: In function 'XvMCExtensionInit':
> | xvmc.c:671:21: warning: the comparison will always evaluate as 'false' for 
> the address of 'XvMCScreenKeyRec' will never be NULL [-Waddress]
> | xvmc.c: In function 'XvMCFindXvImage':
> | xvmc.c:749:22: warning: the comparison will always evaluate as 'false' for 
> the address of 'XvMCScreenKeyRec' will never be NULL [-Waddress]
> 
> Signed-off-by: Cyril Brulebois 
> ---
> Xext/xvmc.c |5 -
> 1 files changed, 0 insertions(+), 5 deletions(-)
> 
> diff --git a/Xext/xvmc.c b/Xext/xvmc.c
> index 4d29941..d49966f 100644
> --- a/Xext/xvmc.c
> +++ b/Xext/xvmc.c
> @@ -668,9 +668,6 @@ XvMCExtensionInit(void)
> {
>ExtensionEntry *extEntry;
> 
> -   if(XvMCScreenKey == NULL) /* nobody supports it */
> - return; 
> -
>if(!(XvMCRTContext = CreateNewResourceType(XvMCDestroyContextRes,
> "XvMCRTContext")))
>   return;
> @@ -746,8 +743,6 @@ XvImagePtr XvMCFindXvImage(XvPortPtr pPort, CARD32 id)
> XvMCAdaptorPtr adaptor = NULL;
> int i;
> 
> -if(XvMCScreenKey == NULL) return NULL;
> -
> if(!(pScreenPriv = XVMC_GET_PRIVATE(pScreen))) 
> return NULL;
> 
> -- 
> 1.7.5.1
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH XTS] xts5: BadDevice takes an int*, not an XID*

2011-05-24 Thread Jeremy Huddleston
Reviewed-by: Jeremy Huddleston 

On May 24, 2011, at 13:03, Aaron Plattner wrote:

> startup.c: In function ‘init_xinput’:
> startup.c:505: error: passing argument 2 of ‘_xibaddevice’ from incompatible 
> pointer type
> /usr/include/X11/extensions/XInput.h:162: note: expected ‘int *’ but argument 
> is of type ‘XID *’
> 
> Signed-off-by: Aaron Plattner 
> ---
> xts5/src/lib/startup.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/xts5/src/lib/startup.c b/xts5/src/lib/startup.c
> index 8f3bcb1..9d4d9bc 100644
> --- a/xts5/src/lib/startup.c
> +++ b/xts5/src/lib/startup.c
> @@ -153,7 +153,7 @@ externint NXI_event;
> int XInputMajorOpcode;
> int XInputFirstError;
> int XInputFirstEvent;
> -XID baddevice;
> +int baddevice;
> 
> Display   *Dsp;
> WindowWin;
> -- 
> 1.7.1
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH XTS] tet: tet_getvar needs to take a const char* argument

2011-05-24 Thread Jeremy Huddleston
Reviewed-by: Jeremy Huddleston 

On May 24, 2011, at 13:18, Aaron Plattner wrote:

> config.c: In function ‘initconfig’:
> config.c:394: error: passing argument 1 of ‘_initconfig’ from incompatible 
> pointer type
> config.c:328: note: expected ‘char * (*)(const char *)’ but argument is of 
> type ‘char * (*)(char *)’
> 
> Signed-off-by: Aaron Plattner 
> ---
> I'm thinking of making a release after this patch, with the current version
> number 0.99.0 even though there are a ton of warning fixes we can still
> make and a lot of the tests are expected to fail.  The reason is that some
> people are actually getting serious about running XTS and I want to give
> them a tagged release version they can use as a baseline.
> 
> Let me know if you have objections, concerns, or changes that you think
> should go in first.
> 
> include/tet_api.h |5 -
> src/tet3/apilib/dconfig.c |5 ++---
> src/tet3/llib/llib-lapi.c |5 ++---
> src/tet3/tcm/dtcm.c   |4 +---
> 4 files changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/include/tet_api.h b/include/tet_api.h
> index 1f70e28..36561ea 100644
> --- a/include/tet_api.h
> +++ b/include/tet_api.h
> @@ -104,6 +104,9 @@ MODIFICATIONS:
>   Andrew Dingwall, UniSoft Ltd., August 1998
>   Added support for shared libraries.
> 
> + Aaron Plattner, NVIDIA Corporation, April 2011
> + tet_getvar needs to take a const char* argument.
> +
> /
> 
> #ifndef TET_API_H_INCLUDED
> @@ -311,7 +314,7 @@ extern "C" {
> /* functions in TETware-Lite and in Distrubuted TETware */
> TET_IMPORT_FUNC(void, tet_delete, TET_PROTOLIST((int, char *)));
> TET_NORETURN TET_IMPORT_FUNC(void, tet_exit, TET_PROTOLIST((int)));
> -TET_IMPORT_FUNC(char *, tet_getvar, TET_PROTOLIST((char *)));
> +TET_IMPORT_FUNC(char *, tet_getvar, TET_PROTOLIST((const char *)));
> TET_IMPORT_FUNC(void, tet_infoline,  TET_PROTOLIST((char *)));
> TET_IMPORT_FUNC(int, tet_kill, TET_PROTOLIST((pid_t, int)));
> TET_IMPORT_FUNC(void, tet_logoff, TET_PROTOLIST((void)));
> diff --git a/src/tet3/apilib/dconfig.c b/src/tet3/apilib/dconfig.c
> index 6306dc2..1295579 100644
> --- a/src/tet3/apilib/dconfig.c
> +++ b/src/tet3/apilib/dconfig.c
> @@ -54,7 +54,7 @@ AUTHOR: Geoff Clare, UniSoft Ltd.
> DATE CREATED: 27 July 1990
> SYNOPSIS:
> 
> - char *tet_getvar(char *name);
> + char *tet_getvar(const char *name);
> 
>   void tet_config(void);
> 
> @@ -106,8 +106,7 @@ MODIFICATIONS:
> static char **varptrs;
> static int lvarptrs, nvarptrs;
> 
> -TET_IMPORT char *tet_getvar(name)
> -char *name;
> +TET_IMPORT char *tet_getvar(const char *name)
> {
>   /* return value of specified configuration variable */
> 
> diff --git a/src/tet3/llib/llib-lapi.c b/src/tet3/llib/llib-lapi.c
> index 949f156..4904bb0 100644
> --- a/src/tet3/llib/llib-lapi.c
> +++ b/src/tet3/llib/llib-lapi.c
> @@ -101,10 +101,9 @@ struct tet_sysent *sysp;
>   return 0;
> }
> 
> -char *tet_getvar(name)
> -char *name;
> +char *tet_getvar(const char *name)
> {
> - return((char *) 0);
> + return((const char *) 0);
> }
> 
> void tet_infoline(data)
> diff --git a/src/tet3/tcm/dtcm.c b/src/tet3/tcm/dtcm.c
> index 331886d..ecfc0a3 100644
> --- a/src/tet3/tcm/dtcm.c
> +++ b/src/tet3/tcm/dtcm.c
> @@ -659,9 +659,7 @@ int sig;
> 
> 
> static void
> -sig_init(var, set)
> -char *var;
> -sigset_t *set;
> +sig_init(const char *var, sigset_t *set)
> {
>   /* initialise signal set from list in specified variable */
> 
> -- 
> 1.7.1
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [Xcb] GPLv3 (Re: [ANNOUNCE] xcb-util 0.3.8)

2011-05-24 Thread Jeremy Huddleston

On May 24, 2011, at 10:17 PM, Arnaud Fontaine wrote:

> However,  xcb/util-wm uses  XCB_UTIL_M4_WITH_INCLUDE_PATH,  so it  still
> depends upon Autoconf 2.62.  Feel free to submit a patch if you have any
> problem with that ;).

My m4 voodoo isn't as good as others', but I'll take a look tomorrow.

But even if I do come up with something that works for you, this just delays 
the inevitable.  This issue will come up again for another feature.  At some 
point we're going to need to have "the license talk" again surrounding our 
build system... 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


1.10.2 delayed

2011-05-28 Thread Jeremy Huddleston
xorg-server-1.10.2 was scheduled for release yesterday, but I'm investigating a 
crash regression that was reported over 1.10.1.

Looking at the code changes, nothing immediately jumps out, but here's the 
backtrace where the reported crash appeared.  I'm getting more information from 
the reporter and will hopefully be able to release 1.10.2 next week.

ShmDestroyPixmap + 23
ChangeWindowAttributes + 811
ProcChangeWindowAttributes + 135
Dispatch + 785


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: 1.10.2 delayed

2011-05-28 Thread Jeremy Huddleston
The user reports that he can't reproduce it now.  1.10.2 will be out shortly.

On May 28, 2011, at 12:46, Jeremy Huddleston wrote:

> xorg-server-1.10.2 was scheduled for release yesterday, but I'm investigating 
> a crash regression that was reported over 1.10.1.
> 
> Looking at the code changes, nothing immediately jumps out, but here's the 
> backtrace where the reported crash appeared.  I'm getting more information 
> from the reporter and will hopefully be able to release 1.10.2 next week.
> 
> ShmDestroyPixmap + 23
> ChangeWindowAttributes + 811
> ProcChangeWindowAttributes + 135
> Dispatch + 785
> 
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH] crash after setting root background pixmap to None, then setting color

2011-05-28 Thread Jeremy Huddleston
I don't see it in your master branch:
http://cgit.freedesktop.org/~daniels/xserver/log/

Your git repo also doesn't seem to know about that commit reference:
http://cgit.freedesktop.org/~daniels/xserver/commit/?id=0701a18b51213ae588cbe82069c0b6eefb8deae7

Also, any thoughts that this could be related to but #31017?
http://bugs.freedesktop.org/show_bug.cgi?id=31017

--Jeremy

On May 25, 2011, at 03:56, Daniel Stone wrote:

> Hi,
> 
> On Mon, May 23, 2011 at 12:26:07PM +0200, Cyril Brulebois wrote:
>> Hi Jeremy,
>> 
>> Marko Macek  (21/05/2011):
>>> Please apply this patch:
>>> 
>>> Fix crash after resetting root window background pixmap to None, then 
>>> setting background color.
>>> 
>>> Signed-off-by: Marko Macek 
> 
> Thanks Marko!
> 
> I have this (committed earlier) as:
> commit 0701a18b51213ae588cbe82069c0b6eefb8deae7
> Author: Marko Macek 
> Date:   Sat May 21 13:30:59 2011 +0100
> 
>DIX: Set backgroundState correctly for root window
> 
>When we change the root window's background to None, and we've run with
>-wr or -br for a forced solid background, make sure we also change the
>background state to BackgroundPixel, so we don't try to lookup either
>pScreen->whitePixel or pScreen->blackPixel as a pixmap.
> 
>Signed-off-by: Marko Macek 
>Reviewed-by: Walter Harms 
>Reviewed-by: Daniel Stone 
> 
> It should be in the master branch of
> git://people.freedesktop.org/~daniels/xserver.
> 
>> it'd be nice to keep an eye on that patch and pick it for the 1.10
>> branch, even though it appears not to be committed to master yet.
>> 
>> Seems to be the same issue as in:
>>  http://bugs.debian.org/626331
>> 
>> A fix for #36986 would also be appreciated for rc3 or final:
>> https://bugs.freedesktop.org/show_bug.cgi?id=36986
>> 
>> (still being discussed in another thread.)
> 
> Yeah, it's a bit of a clanger, and most distros seem to run with either
> -wr or -br, so this would be a great stable candidate.  Let's give it a
> quick run in master and see if it doesn't destroy anything.
> 
> Cheers,
> Daniel
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [ANNOUNCE] xorg-server 1.10.2

2011-05-30 Thread Jeremy Huddleston
Hi Cyril,

My RC2 release notes indicated that I didn't expect to include any changes 
before the final release.  If I had known that these were regressions, I surely 
would've pushed to get these changes into master.  In order to avoid 
communication problems like this in the future, I've updated the wiki with 
instructions on how to communicate "Do Not Release" issues:
http://www.x.org/wiki/Server110Branch


As for specific issues ...


I was under the impression that the "crash after setting root background pixmap 
to None, then setting color" issue was not a regression.  Another user 
contacted me on Friday about this crash (hence the release delay), but he was 
unable to reproduce it reliably on either 1.10.1 nor 1.10.2 RC2.  That code has 
been that way for quite some time, so it seemed to me that this was not a 
regression.  Perhaps something else is now stressing that code-path.  Can you 
confirm that this really is a regression from 1.10.2 and offer a reliable way 
to confirm that it is a regression?

Regarding #36986, I was unfortunately unaware of the scope of the issue.  I was 
under the impression that this was a broken test rather than a broken server.  
I've since CC'd myself on that bug and marked it as a blocker for xserver-1.10.

If these issues are as big as you are making them seem, then I'm inclined to 
shorten the 1.10.3 release cycle.

--Jeremy


On May 30, 2011, at 02:16, Cyril Brulebois wrote:

> Jeremy Huddleston  (28/05/2011):
>> xorg-server-1.10.2 is the second release off of the stable 1.10
>> branch.  Aside from two crash fixes for the XQuartz DDX, it is
>> identical to RC2.  The 1.10 branch is now open for general
>> nominations.
> 
> Sorry, but I wouldn't call that one stable. It introduces a known
> regression from 1.10.1, as seen on:
> 1. https://bugs.freedesktop.org/show_bug.cgi?id=36986
> 2. http://lists.x.org/archives/xorg-devel/2011-May/022455.html
> (aka. <20110523102607.gc2...@mraw.org> if you want to grep your
> maildir)
> 
> Should I have bumped bug importance to critical or blocker? Or done
> anything differently?
> 
> Mraw,
> KiBi.

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [ANNOUNCE] xorg-server 1.10.2

2011-05-30 Thread Jeremy Huddleston
Ok, thanks for confirming that Peter.  I was afraid I had missed something 
important.

On May 30, 2011, at 14:59, Peter Hutterer wrote:

> On Mon, May 30, 2011 at 10:24:37AM -0700, Jeremy Huddleston wrote:
>> Hi Cyril,
>> 
>> My RC2 release notes indicated that I didn't expect to include any changes 
>> before the final release.  If I had known that these were regressions, I 
>> surely would've pushed to get these changes into master.  In order to avoid 
>> communication problems like this in the future, I've updated the wiki with 
>> instructions on how to communicate "Do Not Release" issues:
>> http://www.x.org/wiki/Server110Branch
>> 
>> 
>> As for specific issues ...
>> 
>> 
>> I was under the impression that the "crash after setting root background 
>> pixmap to None, then setting color" issue was not a regression.  Another 
>> user contacted me on Friday about this crash (hence the release delay), but 
>> he was unable to reproduce it reliably on either 1.10.1 nor 1.10.2 RC2.  
>> That code has been that way for quite some time, so it seemed to me that 
>> this was not a regression.  Perhaps something else is now stressing that 
>> code-path.  Can you confirm that this really is a regression from 1.10.2 and 
>> offer a reliable way to confirm that it is a regression?
>> 
>> Regarding #36986, I was unfortunately unaware of the scope of the issue.  I 
>> was under the impression that this was a broken test rather than a broken 
>> server.  I've since CC'd myself on that bug and marked it as a blocker for 
>> xserver-1.10.
> 
> yes, it's a broken test and can be ignored. I got a fair number of comments
> on the patches to fix it but no reviews on the latest patch, so it's not in
> master yet.
> http://patchwork.freedesktop.org/patch/5508/
> Either way, a broken test shouldn't hold up the release.
> 
> Cheers,
>  Peter
> 
>> If these issues are as big as you are making them seem, then I'm inclined to 
>> shorten the 1.10.3 release cycle.
>> 
>> --Jeremy
>> 
>> 
>> On May 30, 2011, at 02:16, Cyril Brulebois wrote:
>> 
>>> Jeremy Huddleston  (28/05/2011):
>>>> xorg-server-1.10.2 is the second release off of the stable 1.10
>>>> branch.  Aside from two crash fixes for the XQuartz DDX, it is
>>>> identical to RC2.  The 1.10 branch is now open for general
>>>> nominations.
>>> 
>>> Sorry, but I wouldn't call that one stable. It introduces a known
>>> regression from 1.10.1, as seen on:
>>> 1. https://bugs.freedesktop.org/show_bug.cgi?id=36986
>>> 2. http://lists.x.org/archives/xorg-devel/2011-May/022455.html
>>>(aka. <20110523102607.gc2...@mraw.org> if you want to grep your
>>>maildir)
>>> 
>>> Should I have bumped bug importance to critical or blocker? Or done
>>> anything differently?
>>> 
>>> Mraw,
>>> KiBi.
>> 
>> 
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH] XQuartz: GLX: Create a new dispatch table rather than modifying the existing one

2011-05-31 Thread Jeremy Huddleston
Ajax, can you please review this.  Will NULL entries in the dispatch table be 
handled properly now, or do we need to initialize the dispatch table with noop 
stubs?

Fixes regression introduced by b0c665ac0fe6840dda581e4d0d0b76c703d62a7b

0   X11.bin 0x000100118293 
__glXAquaScreenCreateContext + 684
1   X11.bin 0x0001001315b0 DoCreateContext + 163
2   X11.bin 0x00010013509f __glXDispatch + 211
3   X11.bin 0x0001000c7dad Dispatch + 785
4   X11.bin 0x0001000b97e5 dix_main + 1022
5   X11.bin 0x0001000122bc server_thread + 50
6   libSystem.B.dylib   0x7fff836554f6 _pthread_start + 331
7   libSystem.B.dylib   0x7fff836553a9 thread_start + 13

http://lists.apple.com/archives/X11-users/2011/May/msg00045.html

Signed-off-by: Jeremy Huddleston 
---
 hw/xquartz/GL/indirect.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/hw/xquartz/GL/indirect.c b/hw/xquartz/GL/indirect.c
index 1375bea..5537973 100644
--- a/hw/xquartz/GL/indirect.c
+++ b/hw/xquartz/GL/indirect.c
@@ -682,7 +682,7 @@ GLuint __glFloorLog2(GLuint val)
 }
 
 static void setup_dispatch_table(void) {
-struct _glapi_table *disp=_glapi_get_dispatch();
+struct _glapi_table *disp=calloc(1,sizeof(struct _glapi_table));
 
 /* to update:
  * for f in $(grep 'define SET_' ../../../glx/dispatch.h  | cut -f2 -d' ' 
| cut -f1 -d\( | sort -u); do grep -q $f indirect.c || echo $f ; done | grep -v 
by_offset | sed 's:SET_\(.*\)$:SET_\1(disp, gl\1)\;:' | pbcopy
@@ -1626,4 +1626,6 @@ static void setup_dispatch_table(void) {
 SET_PixelTexGenParameterivSGIS(disp, glPixelTexGenParameterivSGIS);
 SET_PixelTexGenSGIX(disp, glPixelTexGenSGIX);
 #endif
+
+_glapi_set_dispatch(disp);
 }
-- 
1.7.4.1


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH] test: fix memset size for WindowRec (#37801)

2011-05-31 Thread Jeremy Huddleston
Reviewed-by: Jeremy Huddleston 

On May 31, 2011, at 17:24, Peter Hutterer wrote:

> X.Org Bug 37801 <http://bugs.freedesktop.org/show_bug.cgi?id=37801>
> 
> Signed-off-by: Peter Hutterer 
> ---
> test/xi2/protocol-common.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/test/xi2/protocol-common.c b/test/xi2/protocol-common.c
> index 6ffc697..4234533 100644
> --- a/test/xi2/protocol-common.c
> +++ b/test/xi2/protocol-common.c
> @@ -121,7 +121,7 @@ ClientRec init_client(int len, void *data)
> 
> void init_window(WindowPtr window, WindowPtr parent, int id)
> {
> -memset(window, 0, sizeof(window));
> +memset(window, 0, sizeof(*window));
> 
> window->drawable.id = id;
> if (parent)
> -- 
> 1.7.5.1
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: macros: Changes to 'master'

2011-06-01 Thread Jeremy Huddleston
AC_LANG_DEFINES_PROVIDED seems to be only in newer, GPLv3, versions of autoconf 
... is there a way around this?

autoreconf: Entering directory `.'
autoreconf: configure.ac: not using Gettext
autoreconf: running: aclocal -I /opt/X11/share/aclocal -I 
/usr/local/share/aclocal --force -I m4
autoreconf: configure.ac: tracing
autoreconf: running: glibtoolize --copy --force
glibtoolize: putting auxiliary files in `.'.
glibtoolize: copying file `./ltmain.sh'
glibtoolize: putting macros in `m4'.
glibtoolize: copying file `m4/libtool.m4'
glibtoolize: copying file `m4/ltoptions.m4'
glibtoolize: copying file `m4/ltsugar.m4'
glibtoolize: copying file `m4/ltversion.m4'
glibtoolize: copying file `m4/lt~obsolete.m4'
glibtoolize: Consider adding `AC_CONFIG_MACRO_DIR([m4])' to configure.ac and
glibtoolize: rerunning glibtoolize, to keep the correct libtool macros in-tree.
autoreconf: running: /usr/bin/autoconf --force
configure:14798: error: possibly undefined macro: AC_LANG_DEFINES_PROVIDED
  If this token and others are legitimate, please use m4_pattern_allow.
  See the Autoconf documentation.
autoreconf: /usr/bin/autoconf failed with exit status: 1

On May 25, 2011, at 11:51 PM, Alan Coopersmith wrote:

> xorg-macros.m4.in |6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
> 
> New commits:
> commit e03a5cb9f313c6f5de3edc46327eb18b300b92c2
> Author: Alan Coopersmith 
> Date:   Sat May 14 09:10:13 2011 -0700
> 
>Add AC_LANG_DEFINES_PROVIDED to XORG_RAW_CPP to silence autoconf warnings
> 
>Since we're intentionally doing a special case to just check cpp output,
>and not a full program compilation, add the magic macro to silence:
> 
>configure.ac:46: warning: AC_LANG_CONFTEST: no AC_LANG_SOURCE call 
> detected in body
>../../lib/autoconf/lang.m4:194: AC_LANG_CONFTEST is expanded from...
>aclocal.m4:1077: XORG_PROG_RAWCPP is expanded from...
>configure.ac:46: the top level
> 
>Signed-off-by: Alan Coopersmith 
> 
> ___
> xorg-commit mailing list
> xorg-com...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xorg-commit
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: macros: Changes to 'master'

2011-06-01 Thread Jeremy Huddleston

On Jun 1, 2011, at 6:29 AM, Dan Nicholson wrote:

> On Wed, Jun 1, 2011 at 5:34 AM, Mark Kettenis  wrote:
>> 
>> Many people object to the version 3 of the GPL.  This includes
>> companies like Apple, projects like FreeBSD and OpenBSD, the primary
>> author of the Linux kernel, etc. etc, each for their own reason.  My
>> personal reason is that I don't understand the license anymore.  GPLv2
>> is written in normal language; GPLv3 is full of lawyer speak.
> 
> This is just in the license of the build infrastructure, and doesn't
> extend to the code it's being used with. Do you object to installing
> newer GPLv3 versions of autoconf or are you worried it will infest the
> code?

Just to clarify autoconf's license matrix a bit:

2.61 is the last pure-GPLv2 version
2.62 is the first version that included GPLv3 tools to build autoconf, but the 
installed bits were all GPLv2
2.65 is the first version to be completely GPLv3

In order to use 2.62, I would need to build it.  In order to build it, I would 
have to accept GPLv3 (which is not acceptable).

In order to use 2.65, I would need to accept GPLv3 (which is not acceptable). 

The files generated by autoconf (ie ./configure) are not GPL'd.

If newer autoconf becomes required, I can continue to use the configure scripts 
generated by someone else, but I won't be able to build them myself (ie: I'll 
be forced to use tarballs and patches rather than git).


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


<    2   3   4   5   6   7   8   9   10   11   >