On Wed, Oct 15, 2025 at 02:22:18PM +0530, Avnish Chouhan wrote: > On 2025-10-13 15:37, Michal Suchánek wrote: > > On Mon, Oct 13, 2025 at 03:09:59PM +0530, Avnish Chouhan wrote: > > > This patch adds a check on install_device while installing grub for > > > PowerPC. > > > If install_device is not mentioned in grub2-install and machine is > > > detected > > > as PowerPC, the error will be thrown and it will terminates the > > > grub2-install > > > operation. Running grub2-install on PowerPC without the > > > install_device may > > > result in bootlist corruption. When no install device is specified, > > > it attempts > > > to load images from the filesystem, which leads to nvram bootlist > > > corruption. > > > The idea is to fail the operation and avoid creating the invalid > > > boot entry. > > > > Hi Michal, > Hope you're doing wonderful! > Thank you so much for your response! > > Surely the changes can be done the way you mentioned. But in my honest > opinion, keeping this check in "else" make the code looks lot more > understandable (if we see the complete if else condition). Keeping this in > as added "else" part clearly stats that this check is only specific to > PowerPC. And it has no impact on PowerMac. > Thank you!
As has been pointed out in the other subthread this is in fact confusing. The nesting in an else clause makes this code implicitly non-mac, while reducing the nesting and explicitly checking is_prep makes it clearly non-mac. Thanks Michal > > Regards, > Avnish Chouhan > > > Hello, > > > > there is the part > > if (install_device) > > is_prep = 1; > > > > The check added by this patch now assumes PReP. > > > > For consistency this should be set unconditionally now. > > > > > > > Signed-off-by: Avnish Chouhan <[email protected]> > > > --- > > > util/grub-install.c | 13 +++++++++++++ > > > 1 file changed, 13 insertions(+) > > > > > > diff --git a/util/grub-install.c b/util/grub-install.c > > > index 0602465..f7389b3 100644 > > > --- a/util/grub-install.c > > > +++ b/util/grub-install.c > > > @@ -1289,6 +1289,19 @@ main (int argc, char *argv[]) > > > is_prep = 0; > > > } > > > } > > > +#if defined(__powerpc__) > > > + else > > > + { > > > + /* > > > + * As the machine has been detected as PowerPC and not the > > > PowerMac. We need to check > > > + * whether the install_device has been mentioned while > > > installing. If no device has been > > > + * mentioned, we need to exit and mark it as an error as the > > > install_device is required for > > > + * PowerPC installation. An installation with no device > > > mentioned may lead to corruptions. > > > + */ > > > + if (!install_device) > > Then instead of nesting inside else you could change the condition to > > > > if (is_prep && !install_device) > > > > That clarifies what it is checking for, and will cover the cases when > > there is a partition that is checked for macppc content but none is > > found. > > > > Thanks > > > > Michal > > > + grub_util_error ("%s", _("install device isn't > > > specified required for PowerPC")); > > > > I think this can be worded better. > > > > Something like "No install device specified and no PowerPC Mac partition > > found." > > > > Thanks > > > > Michal > > > > > + } > > > +#endif /* __powerpc__ */ > > > } > > > > > > size_t ndev = 0; > > > -- > > > 2.47.3 > > > _______________________________________________ Grub-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/grub-devel
