On 10/10/2017 03:00 PM, Eric Blake wrote: > On 10/10/2017 09:43 AM, Eric Blake wrote: > >>>> --- >>>> v5: use second label for cleaner exit logic [John], use local_pnum >> >>>> @@ -1811,16 +1811,19 @@ static int64_t coroutine_fn >>>> bdrv_co_get_block_status(BlockDriverState *bs, >>>> int64_t total_sectors; >>>> int64_t n; >>>> int64_t ret, ret2; >>>> + BlockDriverState *local_file = NULL; >>>> + int local_pnum = 0; >>> >>> I don't quite see what the point of local_pnum is if we assert anyway >>> that the real pnum is non-NULL. >> >> I did it in parallel with fallout from John's review on v4: >> https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg06958.html >> >> but since it wasn't specifically asked for, and is now getting >> questions, I'm fine with not having it in v6. > > Okay, I re-read v4, and here's the comment (on 21/23) that led to my > experiment in v5 patch 1 with local_pnum: > > https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg00293.html > > and I did argue: > > https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg00311.html >
Well, Kevin's the boss :D >>> Is it asking for trouble to be updating pnum here before we undo our >>> alignment corrections? For readability reasons and preventing an >>> accidental context-based oopsy-daisy. >> >> As in, write the code to make all calculations in a temporary, and then >> assign *pnum only at the end? I suppose I can tweak the code along >> those lines, but I'm not sure it will make the end result any more legible. >