Re: [PATCH] grub-core/commands/probe.c: Add bootable parameter

[Vladimir 'phcoder' Serbinenko]

On Wed, Dec 13, 2017, 12:30 Carlo Caione <address@hidden> wrote:

On Tue, Dec 12, 2017 at 7:23 PM, Vladimir 'phcoder' Serbinenko
<address@hidden> wrote:
>
>
> On Tue, Dec 12, 2017, 09:56 Carlo Caione <address@hidden> wrote:
>>
>> From: Carlo Caione <address@hidden>
>>
>> Sometimes we are interested in checking whether an msdos partition has
>> the boot flag set. Introduce a new parameter to the probe command to
>> check this.
>>
>> Signed-off-by: Carlo Caione <address@hidden>
>> ---
>>  grub-core/commands/probe.c | 18 ++++++++++++++++++
>>  grub-core/partmap/msdos.c  |  1 +
>>  include/grub/partition.h   |  3 +++
>>  3 files changed, 22 insertions(+)
>>
>> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
>> index cf2793e1d..22d1c5660 100644
>> --- a/grub-core/commands/probe.c
>> +++ b/grub-core/commands/probe.c
>> @@ -31,6 +31,7 @@
>>  #include <grub/env.h>
>>  #include <grub/extcmd.h>
>>  #include <grub/i18n.h>
>> +#include <grub/msdos_partition.h>
>>
>>  GRUB_MOD_LICENSE ("GPLv3+");
>>
>> @@ -45,6 +46,7 @@ static const struct grub_arg_option options[] =
>>      {"fs",             'f', 0, N_("Determine filesystem type."), 0, 0},
>>      {"fs-uuid",                'u', 0, N_("Determine filesystem UUID."),
>> 0, 0},
>>      {"label",          'l', 0, N_("Determine filesystem label."), 0, 0},
>> +    {"bootable",       'b', 0, N_("Determine if bootable / active flag is
>> set."), 0, 0},
>>      {0, 0, 0, 0, 0, 0}
>>    };
>>
>> @@ -154,6 +156,22 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int argc,
>> char **args)
>>        grub_device_close (dev);
>>        return GRUB_ERR_NONE;
>>      }
>> +  if (state[6].set)
>> +    {
>> +      const char *val = "none";
>> +      if (dev->disk &&
>> +          dev->disk->partition &&
>> +          dev->disk->partition->msdostype !=
>> GRUB_PC_PARTITION_TYPE_GPT_DISK &&
>> +          grub_strcmp (dev->disk->partition->partmap->name, "msdos") ==
>> 0)
>> +      if (dev->disk->partition->flag & 0x80)
>> +        val = "bootable";
>> +      if (state[0].set)
>> +        grub_env_set (state[0].arg, val);
>> +      else
>> +        grub_printf ("%s", val);
>> +      grub_device_close (dev);
>> +      return GRUB_ERR_NONE;
>> +    }
>>    grub_device_close (dev);
>>    return grub_error (GRUB_ERR_BAD_ARGUMENT, "unrecognised target");
>>  }
>> diff --git a/grub-core/partmap/msdos.c b/grub-core/partmap/msdos.c
>> index 6d4b455a1..6c13c6b8b 100644
>> --- a/grub-core/partmap/msdos.c
>> +++ b/grub-core/partmap/msdos.c
>> @@ -180,6 +180,7 @@ grub_partition_msdos_iterate (grub_disk_t disk,
>>           p.len = grub_le_to_cpu32 (e->length)
>>             << (disk->log_sector_size - GRUB_DISK_SECTOR_BITS);
>>           p.msdostype = e->type;
>> +         p.flag = e->flag;
>
> part_msdos is too size-sensitive. Please don't add anything there. Just
> reread the sector with MBR or EBR in probing.

the new flag entry is added to struct grub_partition not struct
grub_msdos_partition_mbr, is that actually risky?

I spoke about part_msdos module. It's already larger than it should be

--
Carlo Caione  |  +39.340.80.30.096  |  Endless