From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH v2] mini-os: Fix stubdom build failures on gcc 4.8 Date: Thu, 23 Jan 2014 11:23:40 +0000 Message-ID: <52E0FBBC.1010102@eu.citrix.com> References: <1390412471-12978-1-git-send-email-xenmail43267@gmail.com> <1390475173.24595.49.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1390475173.24595.49.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , xenmail43267@gmail.com Cc: andrew.cooper3@citrix.com, xen-devel@lists.xen.org, Mike Neilsen , stefano.stabellini@citrix.com, samuel.thibault@ens-lyon.org, alex.sharp@orionvm.com List-Id: xen-devel@lists.xenproject.org On 01/23/2014 11:06 AM, Ian Campbell wrote: > On Wed, 2014-01-22 at 11:41 -0600, xenmail43267@gmail.com wrote: >> From: Mike Neilsen >> >> This is a fix for bug 35: >> http://bugs.xenproject.org/xen/bug/35 >> >> This bug report describes several format string mismatches which prevent >> building the stubdom target in Xen 4.3 and Xen 4.4-rc2 on gcc 4.8. This is a >> copy of Alex Sharp's original patch with the following modifications: >> >> * Andrew Cooper's recommendation applied to extras/mini-os/xenbus/xenbus.c to >> avoid stack corruption >> * Samuel Thibault's recommendation to make "fun" an unsigned int rather than an >> unsigned long in pcifront_physical_to_virtual and related functions >> (extras/mini-os/include/pcifront.h and extras/mini-os/pcifront.c) >> >> Tested on x86_64 gcc Ubuntu/Linaro 4.8.1-10ubuntu9. >> >> Signed-off-by: Mike Neilsen >> >> --- >> Changed since v1: >> * Change "fun" arguments into unsigned ints > > Thanks for shaving that yakk! Since you've done it I obviously rescind > my previous comments ;-) (I should have read all my mail first). > > Acked-by: Ian Campbell > > George -- as a build fix for a compiler which is now in the wild I think > this should go in for 4.4. I agree. Release-acked-by: George Dunlap > >> --- >> extras/mini-os/fbfront.c | 4 ++-- >> extras/mini-os/include/pcifront.h | 12 ++++++------ >> extras/mini-os/pcifront.c | 14 +++++++------- >> extras/mini-os/xenbus/xenbus.c | 5 +++-- >> 4 files changed, 18 insertions(+), 17 deletions(-) >> >> diff --git a/extras/mini-os/fbfront.c b/extras/mini-os/fbfront.c >> index 1e01513..9cc07b4 100644 >> --- a/extras/mini-os/fbfront.c >> +++ b/extras/mini-os/fbfront.c >> @@ -105,7 +105,7 @@ again: >> free(err); >> } >> >> - err = xenbus_printf(xbt, nodename, "page-ref","%u", virt_to_mfn(s)); >> + err = xenbus_printf(xbt, nodename, "page-ref","%lu", virt_to_mfn(s)); >> if (err) { >> message = "writing page-ref"; >> goto abort_transaction; >> @@ -468,7 +468,7 @@ again: >> free(err); >> } >> >> - err = xenbus_printf(xbt, nodename, "page-ref","%u", virt_to_mfn(s)); >> + err = xenbus_printf(xbt, nodename, "page-ref","%lu", virt_to_mfn(s)); >> if (err) { >> message = "writing page-ref"; >> goto abort_transaction; >> diff --git a/extras/mini-os/include/pcifront.h b/extras/mini-os/include/pcifront.h >> index 0a6be8e..1b05963 100644 >> --- a/extras/mini-os/include/pcifront.h >> +++ b/extras/mini-os/include/pcifront.h >> @@ -7,23 +7,23 @@ void pcifront_op(struct pcifront_dev *dev, struct xen_pci_op *op); >> void pcifront_scan(struct pcifront_dev *dev, void (*fun)(unsigned int domain, unsigned int bus, unsigned slot, unsigned int fun)); >> int pcifront_conf_read(struct pcifront_dev *dev, >> unsigned int dom, >> - unsigned int bus, unsigned int slot, unsigned long fun, >> + unsigned int bus, unsigned int slot, unsigned int fun, >> unsigned int off, unsigned int size, unsigned int *val); >> int pcifront_conf_write(struct pcifront_dev *dev, >> unsigned int dom, >> - unsigned int bus, unsigned int slot, unsigned long fun, >> + unsigned int bus, unsigned int slot, unsigned int fun, >> unsigned int off, unsigned int size, unsigned int val); >> int pcifront_enable_msi(struct pcifront_dev *dev, >> unsigned int dom, >> - unsigned int bus, unsigned int slot, unsigned long fun); >> + unsigned int bus, unsigned int slot, unsigned int fun); >> int pcifront_disable_msi(struct pcifront_dev *dev, >> unsigned int dom, >> - unsigned int bus, unsigned int slot, unsigned long fun); >> + unsigned int bus, unsigned int slot, unsigned int fun); >> int pcifront_enable_msix(struct pcifront_dev *dev, >> unsigned int dom, >> - unsigned int bus, unsigned int slot, unsigned long fun, >> + unsigned int bus, unsigned int slot, unsigned int fun, >> struct xen_msix_entry *entries, int n); >> int pcifront_disable_msix(struct pcifront_dev *dev, >> unsigned int dom, >> - unsigned int bus, unsigned int slot, unsigned long fun); >> + unsigned int bus, unsigned int slot, unsigned int fun); >> void shutdown_pcifront(struct pcifront_dev *dev); >> diff --git a/extras/mini-os/pcifront.c b/extras/mini-os/pcifront.c >> index 16a4b49..0fc5b30 100644 >> --- a/extras/mini-os/pcifront.c >> +++ b/extras/mini-os/pcifront.c >> @@ -384,7 +384,7 @@ int pcifront_physical_to_virtual (struct pcifront_dev *dev, >> unsigned int *dom, >> unsigned int *bus, >> unsigned int *slot, >> - unsigned long *fun) >> + unsigned int *fun) >> { >> /* FIXME: the buffer sizing is a little lazy here. 10 extra bytes >> should be enough to hold the paths we need to construct, even >> @@ -456,7 +456,7 @@ void pcifront_op(struct pcifront_dev *dev, struct xen_pci_op *op) >> >> int pcifront_conf_read(struct pcifront_dev *dev, >> unsigned int dom, >> - unsigned int bus, unsigned int slot, unsigned long fun, >> + unsigned int bus, unsigned int slot, unsigned int fun, >> unsigned int off, unsigned int size, unsigned int *val) >> { >> struct xen_pci_op op; >> @@ -486,7 +486,7 @@ int pcifront_conf_read(struct pcifront_dev *dev, >> >> int pcifront_conf_write(struct pcifront_dev *dev, >> unsigned int dom, >> - unsigned int bus, unsigned int slot, unsigned long fun, >> + unsigned int bus, unsigned int slot, unsigned int fun, >> unsigned int off, unsigned int size, unsigned int val) >> { >> struct xen_pci_op op; >> @@ -513,7 +513,7 @@ int pcifront_conf_write(struct pcifront_dev *dev, >> >> int pcifront_enable_msi(struct pcifront_dev *dev, >> unsigned int dom, >> - unsigned int bus, unsigned int slot, unsigned long fun) >> + unsigned int bus, unsigned int slot, unsigned int fun) >> { >> struct xen_pci_op op; >> >> @@ -538,7 +538,7 @@ int pcifront_enable_msi(struct pcifront_dev *dev, >> >> int pcifront_disable_msi(struct pcifront_dev *dev, >> unsigned int dom, >> - unsigned int bus, unsigned int slot, unsigned long fun) >> + unsigned int bus, unsigned int slot, unsigned int fun) >> { >> struct xen_pci_op op; >> >> @@ -560,7 +560,7 @@ int pcifront_disable_msi(struct pcifront_dev *dev, >> >> int pcifront_enable_msix(struct pcifront_dev *dev, >> unsigned int dom, >> - unsigned int bus, unsigned int slot, unsigned long fun, >> + unsigned int bus, unsigned int slot, unsigned int fun, >> struct xen_msix_entry *entries, int n) >> { >> struct xen_pci_op op; >> @@ -595,7 +595,7 @@ int pcifront_enable_msix(struct pcifront_dev *dev, >> >> int pcifront_disable_msix(struct pcifront_dev *dev, >> unsigned int dom, >> - unsigned int bus, unsigned int slot, unsigned long fun) >> + unsigned int bus, unsigned int slot, unsigned int fun) >> { >> struct xen_pci_op op; >> >> diff --git a/extras/mini-os/xenbus/xenbus.c b/extras/mini-os/xenbus/xenbus.c >> index ee1691b..c5d9b02 100644 >> --- a/extras/mini-os/xenbus/xenbus.c >> +++ b/extras/mini-os/xenbus/xenbus.c >> @@ -15,6 +15,7 @@ >> * >> **************************************************************************** >> **/ >> +#include >> #include >> #include >> #include >> @@ -672,7 +673,7 @@ char *xenbus_transaction_start(xenbus_transaction_t *xbt) >> err = errmsg(rep); >> if (err) >> return err; >> - sscanf((char *)(rep + 1), "%u", xbt); >> + sscanf((char *)(rep + 1), "%lu", xbt); >> free(rep); >> return NULL; >> } >> @@ -769,7 +770,7 @@ domid_t xenbus_get_self_id(void) >> domid_t ret; >> >> BUG_ON(xenbus_read(XBT_NIL, "domid", &dom_id)); >> - sscanf(dom_id, "%d", &ret); >> + sscanf(dom_id, "%"SCNd16, &ret); >> >> return ret; >> } > >