[Crash-utility] [PATCH 1/1] xendump: Use off_t not long for 32bit code

[Daniel Kiper]

On Wed, Jun 11, 2014 at 02:36:23PM -0400, Dave Anderson wrote:
>
>
> ----- Original Message -----
> > On Wed, Jun 11, 2014 at 01:28:27PM -0400, Don Slutz wrote:
> > > This enables crash to handle xen dumps that are larger then 4G
> > > in size in 32bit mode.
> > >
> > > Signed-off-by: Don Slutz <dslutz at verizon.com>
> > > ---
> > > This is the same as was sent as an attachment.  Just a clean top post.
> > >
> > >  x86.c     | 10 ++++-----
> > >  x86_64.c  | 10 ++++-----
> > >  xendump.c | 74
> > >  +++++++++++++++++++++++++++++++++------------------------------
> > >  xendump.h |  6 +++---
> > >  4 files changed, 52 insertions(+), 48 deletions(-)
> > >
> > > diff --git a/x86.c b/x86.c
> > > index 833a11b..608bb88 100644
> > > --- a/x86.c
> > > +++ b/x86.c
> > > @@ -4897,7 +4897,7 @@ x86_xendump_p2m_create(struct xendump_data *xd)
> > >  		    "MEMBER_OFFSET(vcpu_guest_context, ctrlreg): %ld\n",
> > >  			ctrlreg_offset);
> > >
> > > -	offset = (off_t)xd->xc_core.header.xch_ctxt_offset +
> > > +	offset = xd->xc_core.header.xch_ctxt_offset +
> > >  		(off_t)ctrlreg_offset;
> >
> > Good job. However, as I can see this fix requires explicit type
> > conversions in many places. I am almost sure that next time when
> > somebody improve the code then he/she will forget about this needed
> > type conversions. So maybe it is worth creating new struct which
> > will have xch_ctxt_offset as off_t instead of unsigned long and
> > do this conversion once somewhere.
> >
> > Daniel
>
> Hi Daniel,
>
> I'm confused -- isn't that exactly what Don has done?  I.e., changing the
> type of the xen_core_header's xch_ctxt_offset, xch_index_offset and
> xch_pages_offset members from unsigned long to off_t.  It does require
> explicit type conversions when assigning them from the obsoleted
> xenctrl.h xc_core_header structures where they were/are unsigned ints,
> and when assigning them from shdr.sh_offset values, but for the most
> part, the patch mostly removes the (off_t) casts.

Ugh... Sorry... What a shame... Forget it... I was reading in my
imagination not what was written...

> I don't have any sample 32-bit Xen ELF coredumps larger than 4GB,

I do not have either.

> and our support organization has never filed any bugs reporting
> this issue.  (Could it require later versions of the dom0 kernel
> or other Xen sources?)  Anyway, the patch seems safe enough, and

This issue was discovered by guy from Huawei on Xen 4.1.2.
I will ask him to do a test.

> doesn't break any of my other sample x86 and x86_64 Xen dumpfiles.

I agree.

Daniel