On Thu, 2012-01-12 at 13:49 -0800, Darren Hart wrote: > > On 01/12/2012 01:47 PM, Tom Zanussi wrote: > > On Thu, 2012-01-12 at 13:24 -0800, Darren Hart wrote: > >> > >> On 01/12/2012 10:33 AM, Tom Zanussi wrote: > >>> A couple of things that had previously been warnings are now errors, > >>> so they need to be fixed up. > >>> > >>> The first problem is a comparison between the address of a static > >>> struct and NULL, which can never be valid. A different fix for this > >>> is upstream, which includes an API usage change; we don't need that to > >>> fix this problem. > >>> > >>> The second problem is a cast from pointer to integer in fbdevhw.c. > >>> This also is fixed upstream by removing the whole section of code > >>> which is bogus anyway, which is also done here. > >>> > >>> This also adds a missing PR to the xserver-xorg recipe. > >>> > >>> Signed-off-by: Tom Zanussi <[email protected]> > >> > >> I'm fine with the patch given your upstream API change comment above. I > >> am curious about just dropping the struct == NULL test as opposed to > >> testing for some value in the structure that would indicate the intended > >> condition. If you're happy with it as is, then: > >> > > > > Well, the struct == NULL test can obviously never affect anything at > > run-time, and by definition never has, so can be safely removed. > > > > Changing the code to actually test something inside the struct that may > > or may not be meaningful would mean auditing all the code to find out > > whether the contents are always in a sane state for that test. > > Ah, yes, good point. Looks good to me as is. >
OK, thanks for the ack.. Tom > -- > Darren > > > > > Given the choice between a fix that makes no run-time changes and has no > > possibility of causing a regression vs spending a lot of time making an > > old version of obviously buggy code I know nothing about perfect, I > > chose the former. While I'm at it, I should also probably fix the 7000 > > other warnings - the only reason I'm in here in the first place is that > > a couple of them suddenly turned into errors... > > > > Tom > > > >> Acked-by: Darren Hart <[email protected]> > >> > >>> --- > >>> .../xorg-xserver/xserver-xorg-1.9.3.inc | 5 + > >>> .../xserver-xorg-1.9.3/ptr-to-int-cast-fix.patch | 92 > >>> ++++++++++++++++++++ > >>> .../xserver-xorg-1.9.3/werror-address-fix.patch | 49 +++++++++++ > >>> 3 files changed, 146 insertions(+), 0 deletions(-) > >>> create mode 100644 > >>> common/recipes-graphics/xorg-xserver/xserver-xorg-1.9.3/ptr-to-int-cast-fix.patch > >>> create mode 100644 > >>> common/recipes-graphics/xorg-xserver/xserver-xorg-1.9.3/werror-address-fix.patch > >>> > >>> diff --git a/common/recipes-graphics/xorg-xserver/xserver-xorg-1.9.3.inc > >>> b/common/recipes-graphics/xorg-xserver/xserver-xorg-1.9.3.inc > >>> index 888445d..8c7009f 100644 > >>> --- a/common/recipes-graphics/xorg-xserver/xserver-xorg-1.9.3.inc > >>> +++ b/common/recipes-graphics/xorg-xserver/xserver-xorg-1.9.3.inc > >>> @@ -4,6 +4,10 @@ SRC_URI += "file://nodolt.patch \ > >>> # Misc build failure for master HEAD > >>> SRC_URI += "file://fix_open_max_preprocessor_error.patch" > >>> > >>> +# What once were warnings now are errors, fix those up > >>> +SRC_URI += "file://werror-address-fix.patch \ > >>> + file://ptr-to-int-cast-fix.patch" > >>> + > >>> PROTO_DEPS += "xf86driproto dri2proto" > >>> DEPENDS += "font-util" > >>> EXTRA_OECONF += "--enable-dri --enable-dri2 --enable-dga" > >>> @@ -13,3 +17,4 @@ LIC_FILES_CHKSUM = > >>> "file://COPYING;md5=3dd2bbe3563837f80ed8926b06c1c353" > >>> SRC_URI[md5sum] = "5bef6839a76d029204ab31aa2fcb5201" > >>> SRC_URI[sha256sum] = > >>> "864831f51e841ff37f2445d1c85b86b559c8860a435fb496aead4f256a2b141d" > >>> > >>> +PR = "r1" > >>> diff --git > >>> a/common/recipes-graphics/xorg-xserver/xserver-xorg-1.9.3/ptr-to-int-cast-fix.patch > >>> > >>> b/common/recipes-graphics/xorg-xserver/xserver-xorg-1.9.3/ptr-to-int-cast-fix.patch > >>> new file mode 100644 > >>> index 0000000..705cffc > >>> --- /dev/null > >>> +++ > >>> b/common/recipes-graphics/xorg-xserver/xserver-xorg-1.9.3/ptr-to-int-cast-fix.patch > >>> @@ -0,0 +1,92 @@ > >>> +Upstream-Status: Inappropriate [already upstream] > >>> + > >>> +It's broken for devices with BARs above 4G, and the sysfs method should > >>> > >>> +work everywhere anyway. As a pleasant side effect, this fixes some > >>> > >>> +warnings: > >>> > >>> + > >>> > >>> +fbdevhw.c: In function 'fbdev_open_pci': > >>> > >>> +fbdevhw.c:333:4: warning: cast from pointer to integer of different size > >>> > >>> +fbdevhw.c:334:4: warning: cast from pointer to integer of different size > >>> > >>> +fbdevhw.c:336:4: warning: cast from pointer to integer of different size > >>> > >>> +fbdevhw.c:337:4: warning: cast from pointer to integer of different size > >>> > >>> + > >>> > >>> +Signed-off-by: Adam Jackson <ajax (a] redhat.com> > >>> > >>> +Integrated-by: Tom Zanussi <tom.zanussi (a] intel.com> > >>> + > >>> +Index: xorg-server-1.9.3/hw/xfree86/fbdevhw/fbdevhw.c > >>> +=================================================================== > >>> +--- xorg-server-1.9.3.orig/hw/xfree86/fbdevhw/fbdevhw.c 2012-01-12 > >>> 10:32:07.097729262 -0600 > >>> ++++ xorg-server-1.9.3/hw/xfree86/fbdevhw/fbdevhw.c 2012-01-12 > >>> 10:32:55.076732780 -0600 > >>> +@@ -291,14 +291,7 @@ > >>> + { > >>> + struct fb_fix_screeninfo fix; > >>> + char filename[256]; > >>> +- int fd,i,j; > >>> +- > >>> +- > >>> +- /* There are two ways to that we can determine which fb device is > >>> +- * associated with this PCI device. The more modern way is to look > >>> in > >>> +- * the sysfs directory for the PCI device for a file named > >>> +- * "graphics/fb*" > >>> +- */ > >>> ++ int fd, i; > >>> + > >>> + for (i = 0; i < 8; i++) { > >>> + sprintf(filename, > >>> +@@ -331,55 +324,10 @@ > >>> + } > >>> + } > >>> + > >>> +- > >>> +- /* The other way is to examine the resources associated with each fb > >>> +- * device and see if there is a match with the PCI device. This > >>> technique > >>> +- * has some problems on certain mixed 64-bit / 32-bit architectures. > >>> +- * There is a flaw in the fb_fix_screeninfo structure in that it > >>> only > >>> +- * returns the low 32-bits of the address of the resources > >>> associated with > >>> +- * a device. However, on a mixed architecture the base addresses > >>> of PCI > >>> +- * devices, even for 32-bit applications, may be higher than > >>> 0x0f0000000. > >>> +- */ > >>> +- > >>> +- for (i = 0; i < 8; i++) { > >>> +- sprintf(filename,"/dev/fb%d",i); > >>> +- if (-1 == (fd = open(filename,O_RDWR,0))) { > >>> +- xf86DrvMsg(-1, X_WARNING, > >>> +- "open %s: %s\n", filename, strerror(errno)); > >>> +- continue; > >>> +- } > >>> +- if (-1 == ioctl(fd,FBIOGET_FSCREENINFO,(void*)&fix)) { > >>> +- close(fd); > >>> +- continue; > >>> +- } > >>> +- for (j = 0; j < 6; j++) { > >>> +- const pciaddr_t res_start = pPci->regions[j].base_addr; > >>> +- const pciaddr_t res_end = res_start + pPci->regions[j].size; > >>> +- > >>> +- if ((0 != fix.smem_len && > >>> +- (pciaddr_t) fix.smem_start >= res_start && > >>> +- (pciaddr_t) fix.smem_start < res_end) || > >>> +- (0 != fix.mmio_len && > >>> +- (pciaddr_t) fix.mmio_start >= res_start && > >>> +- (pciaddr_t) fix.mmio_start < res_end)) > >>> +- break; > >>> +- } > >>> +- if (j == 6) { > >>> +- close(fd); > >>> +- continue; > >>> +- } > >>> +- if (namep) { > >>> +- *namep = xnfalloc(16); > >>> +- strncpy(*namep,fix.id,16); > >>> +- } > >>> +- return fd; > >>> +- } > >>> +- > >>> + if (namep) > >>> + *namep = NULL; > >>> + > >>> +- xf86DrvMsg(-1, X_ERROR, > >>> +- "Unable to find a valid framebuffer device\n"); > >>> ++ xf86DrvMsg(-1, X_ERROR, "Unable to find a valid framebuffer > >>> device\n"); > >>> + return -1; > >>> + } > >>> + > >>> diff --git > >>> a/common/recipes-graphics/xorg-xserver/xserver-xorg-1.9.3/werror-address-fix.patch > >>> > >>> b/common/recipes-graphics/xorg-xserver/xserver-xorg-1.9.3/werror-address-fix.patch > >>> new file mode 100644 > >>> index 0000000..49d3f94 > >>> --- /dev/null > >>> +++ > >>> b/common/recipes-graphics/xorg-xserver/xserver-xorg-1.9.3/werror-address-fix.patch > >>> @@ -0,0 +1,49 @@ > >>> +Upstream-Status: Inappropriate [yocto-specific] > >>> + > >>> +This is fixed upstream by actually making these tests meaningful. > >>> +As they stand, the warning is correct and they're no-ops, so remove > >>> +them. > >>> + > >>> +Signed-off-by: Tom Zanussi <tom.zanussi (a] intel.com> > >>> + > >>> +Index: xorg-server-1.9.3/Xext/xvmc.c > >>> +=================================================================== > >>> +--- xorg-server-1.9.3.orig/Xext/xvmc.c 2012-01-12 09:57:36.306947860 > >>> -0600 > >>> ++++ xorg-server-1.9.3/Xext/xvmc.c 2012-01-12 10:24:59.286729946 > >>> -0600 > >>> +@@ -467,7 +467,6 @@ > >>> + return Success; > >>> + } > >>> + > >>> +- > >>> + static int > >>> + ProcXvMCListSubpictureTypes(ClientPtr client) > >>> + { > >>> +@@ -487,9 +486,6 @@ > >>> + > >>> + pScreen = pPort->pAdaptor->pScreen; > >>> + > >>> +- if(XvMCScreenKey == NULL) /* No XvMC adaptors */ > >>> +- return BadMatch; > >>> +- > >>> + if(!(pScreenPriv = XVMC_GET_PRIVATE(pScreen))) > >>> + return BadMatch; /* None this screen */ > >>> + > >>> +@@ -668,9 +664,6 @@ > >>> + { > >>> + ExtensionEntry *extEntry; > >>> + > >>> +- if(XvMCScreenKey == NULL) /* nobody supports it */ > >>> +- return; > >>> +- > >>> + if(!(XvMCRTContext = CreateNewResourceType(XvMCDestroyContextRes, > >>> + "XvMCRTContext"))) > >>> + return; > >>> +@@ -746,8 +739,6 @@ > >>> + XvMCAdaptorPtr adaptor = NULL; > >>> + int i; > >>> + > >>> +- if(XvMCScreenKey == NULL) return NULL; > >>> +- > >>> + if(!(pScreenPriv = XVMC_GET_PRIVATE(pScreen))) > >>> + return NULL; > >>> + > >> > > > > > _______________________________________________ yocto mailing list [email protected] https://lists.yoctoproject.org/listinfo/yocto
