Am 08.10.2015 um 18:44 hat John Snow geschrieben: > On 10/08/2015 08:06 AM, Peter Lieven wrote: > > Hi all, > > > > short summary from my side. The whole thing seems to get complicated, > > let me explain why: > > > > 1) During review I found that the code in ide_atapi_cmd_reply_end can't > > work correctly if the > > byte_count_limit is not a divider or a multiple of cd_sector_size. The > > reason is that as soon > > as we load the next sector we start at io_buffer offset 0 overwriting > > whatever is left in there > > for transfer. We also reset the io_buffer_index to 0 which means if we > > continue with the > > elementary transfer we always transfer a whole sector (of corrupt data) > > regardless if we > > are allowed to transfer that much data. Before we consider fixing this I > > wonder if it > > is legal at all to have an unaligned byte_count_limit. It obviously has > > never caused trouble in > > practice so maybe its not happening in real life. > > > > I had overlooked that part. Good catch. I do suspect that in practice > nobody will be asking for bizarre values. > > There's no rule against an unaligned byte_count_limit as far as I have > read, but suspect nobody would have a reason to use it in practice.
If we know that our code is technically wrong, "nobody uses it" is not a good reason not to fix it. Because sooner or later someone will use it. > > 2) I found that whatever cool optimization I put in to buffer multiple > > sectors at once I end > > up with code that breaks migration because older versions would either > > not fill the io_buffer > > as expected or we introduce variables that older versions do not > > understand. This will > > lead to problems if we migrate in the middle of a transfer. > > > > Ech. This sounds like a bit of a problem. I'll need to think about this > one... Sounds like a textbook example for subsections to me? Kevin
