John Snow <js...@redhat.com> writes:
> Modify this function to operate directly on FDrive objects instead of
> unpacking and passing all of those parameters manually.
>
> Helps reduce complexity in each caller, and reduces the number of args.
For now, there's just one.
Diffstat suggests it's an overall simplification.
> Signed-off-by: John Snow <js...@redhat.com>
> ---
> hw/block/fdc.c | 54 +++++++++++++++++++++++-------------------------------
> 1 file changed, 23 insertions(+), 31 deletions(-)
>
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 246b631..09bb63d 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -241,11 +241,9 @@ static void fd_recalibrate(FDrive *drv)
> fd_seek(drv, 0, 0, 1, 1);
> }
>
> -static void pick_geometry(BlockBackend *blk, int *nb_heads,
> - int *max_track, int *last_sect,
> - FDriveType drive_in, FDriveType *drive,
> - FDriveRate *rate)
> +static void pick_geometry(FDrive *drv)
> {
> + BlockBackend *blk = drv->blk;
> const FDFormat *parse;
> uint64_t nb_sectors, size;
> int i, first_match, match;
> @@ -258,8 +256,8 @@ static void pick_geometry(BlockBackend *blk, int
> *nb_heads,
> if (parse->drive == FDRIVE_DRV_NONE) {
> break;
> }
> - if (drive_in == parse->drive ||
> - drive_in == FDRIVE_DRV_NONE) {
> + if (drv->drive == parse->drive ||
> + drv->drive == FDRIVE_DRV_NONE) {
> size = (parse->max_head + 1) * parse->max_track *
> parse->last_sect;
> if (nb_sectors == size) {
> @@ -279,41 +277,35 @@ static void pick_geometry(BlockBackend *blk, int
> *nb_heads,
> }
> parse = &fd_formats[match];
> }
> - *nb_heads = parse->max_head + 1;
> - *max_track = parse->max_track;
> - *last_sect = parse->last_sect;
> - *drive = parse->drive;
> - *rate = parse->rate;
> +
> + if (parse->max_head == 0) {
> + drv->flags &= ~FDISK_DBL_SIDES;
> + } else {
> + drv->flags |= FDISK_DBL_SIDES;
> + }
> + drv->max_track = parse->max_track;
> + drv->last_sect = parse->last_sect;
> + drv->drive = parse->drive;
> + drv->media_rate = parse->rate;
> +
> + if (drv->media_inserted) {
> + FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n",
> + parse->max_head + 1,
> + drv->max_track, drv->last_sect,
> + drv->ro ? "ro" : "rw");
> + }
One half of the debug print moved from...
> }
>
> /* Revalidate a disk drive after a disk change */
> static void fd_revalidate(FDrive *drv)
> {
> - int nb_heads, max_track, last_sect, ro;
> - FDriveType drive;
> - FDriveRate rate;
> -
> FLOPPY_DPRINTF("revalidate\n");
> if (drv->blk != NULL) {
> - ro = blk_is_read_only(drv->blk);
> - pick_geometry(drv->blk, &nb_heads, &max_track,
> - &last_sect, drv->drive, &drive, &rate);
> + drv->ro = blk_is_read_only(drv->blk);
> + pick_geometry(drv);
> if (!drv->media_inserted) {
> FLOPPY_DPRINTF("No disk in drive\n");
> - } else {
> - FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n", nb_heads,
> - max_track, last_sect, ro ? "ro" : "rw");
> }
... here. Recommend to move both or none. If you add more callers,
both might make more sense.
> - if (nb_heads == 1) {
> - drv->flags &= ~FDISK_DBL_SIDES;
> - } else {
> - drv->flags |= FDISK_DBL_SIDES;
> - }
> - drv->max_track = max_track;
> - drv->last_sect = last_sect;
> - drv->ro = ro;
> - drv->drive = drive;
> - drv->media_rate = rate;
> } else {
> FLOPPY_DPRINTF("No drive connected\n");
> drv->last_sect = 0;
