Re: [SeaBIOS] [PATCH] pretty boot menu entry for cdrom drives

[Kevin O'Connor]

On Fri, Sep 21, 2018 at 09:48:01AM +0200, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann
>
> src/util.h  |  1 +
>  src/boot.c  |  7 +++++++
>  src/cdrom.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 59 insertions(+)
>
> diff --git a/src/util.h b/src/util.h
> index 7a23b518fc..6dd080f673 100644
> --- a/src/util.h
> +++ b/src/util.h
> @@ -50,6 +50,7 @@ struct disk_op_s;
>  int cdemu_process_op(struct disk_op_s *op);
>  void cdrom_prepboot(void);
>  int cdrom_boot(struct drive_s *drive_g);
> +char *cdrom_media_info(struct drive_s *drive_g);
>
> // clock.c
>  void clock_setup(void);
> diff --git a/src/boot.c b/src/boot.c
> index ff705fd47f..80bcc13535 100644
> --- a/src/boot.c
> +++ b/src/boot.c
> @@ -395,6 +395,13 @@ boot_add_hd(struct drive_s *drive, const char *desc, int prio)
>  void
>  boot_add_cd(struct drive_s *drive, const char *desc, int prio)
>  {
> if (GET_GLOBAL(PlatformRunningOn) & PF_QEMU) {
>    // We want short boot times.  But on physical hardware even
>    // the test unit ready can take several seconds.  So do media
>    // access on qemu only, where we know it will be fast.
>    char *extra = cdrom_media_info(drive);
>    desc = znprintf(MAXDESCSIZE, "%s (%s)", desc, extra);
> }
> bootentry_add(IPL_TYPE_CDROM, defPrio(prio, DefaultCDPrio)
>               , (u32)drive, desc);
>
> }
> diff --git a/src/cdrom.c b/src/cdrom.c
> index 828fb3b842..b4c36622ae 100644
> --- a/src/cdrom.c
> +++ b/src/cdrom.c
> @@ -274,3 +274,54 @@ cdrom_boot(struct drive_s *drive)
>  return 0;
>
> }
>
> +// go figure some cdrom information, for a pretty boot menu entry.
> +char*
> +cdrom_media_info(struct drive_s *drive)
> +{
> ASSERT32FLAT();
> struct disk_op_s dop;
> memset(&dop, 0, sizeof(dop));
> dop.drive_fl = drive;
> int ret = scsi_is_ready(&dop);
> if (ret)
>    return "empty";
> // Read the Boot Record Volume Descriptor
> u8 buffer[CDROM_SECTOR_SIZE];
> dop.command = CMD_READ;
> dop.lba = 0x11;
> dop.count = 1;
> dop.buf_fl = buffer;
> ret = process_op(&dop);
> if (ret)
>    return "read error";
> // Is it bootable?
> if (buffer[0])
>    return "not bootable";
> if (strcmp((char*)&buffer[1], "CD001\001EL TORITO SPECIFICATION") != 0)
>    return "not bootable";
> // Read the Primary Volume Descriptor
> dop.command = CMD_READ;
> dop.lba = 0x10;
> dop.count = 1;
> dop.buf_fl = buffer;
> ret = process_op(&dop);
> if (ret)
>    return "read error";
> // Read volume id, trim trailing spaces
> char *volume = znprintf(30, "%s", buffer + 40);
> char *h = volume + strlen(volume);
> while (h > volume) {
>    h--;
>    if (*h != ' ')
>        break;
>    *h = 0;
> }
> return volume;
>
> +}

Thanks.  In general, looks fine to me.

Some minor comments:

If there's a read error or non-bootable device, shouldn't it just
revert to the original description?

There's a nullTrailingSpace() function to remove trailing spaces.

-Kevin