[yocto] [PATCH] meta-intel/common: fix ptr->int and Werror=address compile errors
Darren Hart
dvhart at linux.intel.com
Thu Jan 12 13:49:57 PST 2012
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 <tom.zanussi at intel.com>
>>
>> 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.
--
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 <dvhart at linux.intel.com>
>>
>>> ---
>>> .../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;
>>> +
>>
>
>
--
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel
More information about the yocto
mailing list