Yes, this patch doesn't make the code worse. On 06/27/2014 05:11 PM, Christian Borntraeger wrote: > On 27/06/14 15:04, Eugene "jno" Dvurechenski wrote: >> >> >> On 06/27/2014 03:55 PM, Christian Borntraeger wrote: >>>>> - const int max_entries = (SECTOR_SIZE / sizeof(ScsiBlockPtr)); >>>>> + const int max_entries = (MAX_SECTOR_SIZE / sizeof(ScsiBlockPtr)); >>>> >>>> Is this really safe to increase? Doesn't max_entries depend on the real >>>> sector size? >>> >>> I think this is now covered by this if statement: >>> if (bprs[i].blockct == 0 && unused_space(&bprs[i + 1], >>> sizeof(ScsiBlockPtr))) { >>> >>> which was introduced by commit c77cd87cf54f003748f29c14ea1ddaecfc5c653f >>> (pc-bios/s390-ccw: fix for fragmented SCSI bootmap). >>> >>> So strictly speaking this if statement might not be needed any more: >>> if (i == (max_entries - 1)) { >>> >>> Eugene, can you confirm? If yes we could add this patch later on as a >>> cleanup: >> >> I'd preserve both checks. >> In theory, we may catch a table that consumes all scratch space and >> leave no unused entry. >> >> Plus, this check for zero counter and last entry is for "continuation" >> pointer, not for end-of-table by itself. >> >> I think now, this code may need even few more checks to cover more cases... >> > Ok. That means, that this patch as is, doesnt make anything worse. Correct? > > I am expecting more fixes and cleanups for the bios code anyway, so as long > as we dont add a regression here this should be good to go as it makes the > whole code more flexible. > > Christian >
-- Best Regards, Eugene "jno" Dvurechenski zLinux (KVM) Development - Software Engineer IBM Russia - Science & Technology Center phone: +7 (495) 660 8940 ext. 1021
<<attachment: jno.vcf>>
signature.asc
Description: OpenPGP digital signature