Re: [qemu-s390x] [PATCH v4 02/10] s390-ccw: refactor eckd_block_num to use CHS

[Cornelia Huck]

On Thu, 25 Jan 2018 12:06:50 +0100
Thomas Huth <address@hidden> wrote:

> On 23.01.2018 19:26, Collin L. Walling wrote:
> > Add new cylinder/head/sector struct. Use it to calculate
> > eckd block numbers instead of a BootMapPointer (which used
> > eckd chs anyway).
> > 
> > Signed-off-by: Collin L. Walling <address@hidden>
> > ---
> >  pc-bios/s390-ccw/bootmap.c | 28 ++++++++++++++--------------
> >  pc-bios/s390-ccw/bootmap.h |  8 ++++++--
> >  2 files changed, 20 insertions(+), 16 deletions(-)
> > 
> > diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
> > index 6b6c915..621adbe 100644
> > --- a/pc-bios/s390-ccw/bootmap.c
> > +++ b/pc-bios/s390-ccw/bootmap.c
> > @@ -95,32 +95,32 @@ static inline void verify_boot_info(BootInfo *bip)
> >                 "Bad block size in zIPL section of the 1st record.");
> >  }
> >  
> > -static block_number_t eckd_block_num(BootMapPointer *p)
> > +static block_number_t eckd_block_num(EckdCHS chs)  
> 
> Should this maybe rather be call-by-pointer instead? I'm not a fan of
> passing structs by value, though it might be OK in this case since it's
> a small struct only...
> 
> What do others think?

I think passing a struct by value is fine for things like a schid
(which is basically just the structured version of an integer). In this
case, I think passing a pointer would look nicer.