| Message ID | 20200715094045.381984-5-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pc-bios: s390x: Cleanup part 2 | expand |
On 15/07/2020 11.40, Janosch Frank wrote: > Sometimes a memset is nicer to read than multiple struct->data = 0; > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > Reviewed-by: Pierre Morel <pmorel@linux.ibm.com> > --- > pc-bios/s390-ccw/dasd-ipl.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/pc-bios/s390-ccw/dasd-ipl.c b/pc-bios/s390-ccw/dasd-ipl.c > index e8f2846740..0543334ed4 100644 > --- a/pc-bios/s390-ccw/dasd-ipl.c > +++ b/pc-bios/s390-ccw/dasd-ipl.c > @@ -167,16 +167,13 @@ static void ipl1_fixup(void) > ccwSeek->cda = ptr2u32(seekData); > ccwSeek->chain = 1; > ccwSeek->count = sizeof(*seekData); > - seekData->reserved = 0x00; > - seekData->cyl = 0x00; > - seekData->head = 0x00; > + memset(seekData, 0, sizeof(*seekData)); Sounds ok for me if the whole struct gets cleared (though I wonder whether this is really worth the effort)... > ccwSearchID->cmd_code = CCW_CMD_DASD_SEARCH_ID_EQ; > ccwSearchID->cda = ptr2u32(searchData); > ccwSearchID->chain = 1; > ccwSearchID->count = sizeof(*searchData); > - searchData->cyl = 0; > - searchData->head = 0; > + memset(searchData, 0, sizeof(*searchData)); > searchData->record = 2; ... but that looks rather worse to me, and the generated code will likely also be slightly worse (since ->record is cleared first and then set to 2 again). Maybe rather drop this patch? Thomas
On 7/20/20 1:56 PM, Thomas Huth wrote: > On 15/07/2020 11.40, Janosch Frank wrote: >> Sometimes a memset is nicer to read than multiple struct->data = 0; >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> Reviewed-by: Pierre Morel <pmorel@linux.ibm.com> >> --- >> pc-bios/s390-ccw/dasd-ipl.c | 7 ++----- >> 1 file changed, 2 insertions(+), 5 deletions(-) >> >> diff --git a/pc-bios/s390-ccw/dasd-ipl.c b/pc-bios/s390-ccw/dasd-ipl.c >> index e8f2846740..0543334ed4 100644 >> --- a/pc-bios/s390-ccw/dasd-ipl.c >> +++ b/pc-bios/s390-ccw/dasd-ipl.c >> @@ -167,16 +167,13 @@ static void ipl1_fixup(void) >> ccwSeek->cda = ptr2u32(seekData); >> ccwSeek->chain = 1; >> ccwSeek->count = sizeof(*seekData); >> - seekData->reserved = 0x00; >> - seekData->cyl = 0x00; >> - seekData->head = 0x00; >> + memset(seekData, 0, sizeof(*seekData)); > > Sounds ok for me if the whole struct gets cleared (though I wonder > whether this is really worth the effort)... > >> ccwSearchID->cmd_code = CCW_CMD_DASD_SEARCH_ID_EQ; >> ccwSearchID->cda = ptr2u32(searchData); >> ccwSearchID->chain = 1; >> ccwSearchID->count = sizeof(*searchData); >> - searchData->cyl = 0; >> - searchData->head = 0; >> + memset(searchData, 0, sizeof(*searchData)); >> searchData->record = 2; > > ... but that looks rather worse to me, and the generated code will > likely also be slightly worse (since ->record is cleared first and then > set to 2 again). > > Maybe rather drop this patch? Sure, I'm definitely not hard set on this patch :) > > Thomas > >
diff --git a/pc-bios/s390-ccw/dasd-ipl.c b/pc-bios/s390-ccw/dasd-ipl.c index e8f2846740..0543334ed4 100644 --- a/pc-bios/s390-ccw/dasd-ipl.c +++ b/pc-bios/s390-ccw/dasd-ipl.c @@ -167,16 +167,13 @@ static void ipl1_fixup(void) ccwSeek->cda = ptr2u32(seekData); ccwSeek->chain = 1; ccwSeek->count = sizeof(*seekData); - seekData->reserved = 0x00; - seekData->cyl = 0x00; - seekData->head = 0x00; + memset(seekData, 0, sizeof(*seekData)); ccwSearchID->cmd_code = CCW_CMD_DASD_SEARCH_ID_EQ; ccwSearchID->cda = ptr2u32(searchData); ccwSearchID->chain = 1; ccwSearchID->count = sizeof(*searchData); - searchData->cyl = 0; - searchData->head = 0; + memset(searchData, 0, sizeof(*searchData)); searchData->record = 2; /* Go back to Search CCW if correct record not yet found */