[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] trivial fixes for windows-efi branch
From: |
Vladimir 'φ-coder/phcoder' Serbinenko |
Subject: |
Re: [PATCH] trivial fixes for windows-efi branch |
Date: |
Sun, 15 Dec 2013 17:43:28 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20131103 Icedove/17.0.10 |
On 15.12.2013 13:41, Andrey Borzenkov wrote:
> В Sun, 15 Dec 2013 12:43:49 +0100
> "Vladimir 'phcoder' Serbinenko" <address@hidden> пишет:
>
>> Sounds like you're looking at old version. I've fixed those problems and
>> it's already merged in master. Could you have a look and say if any of
>> those still a problem?
>
> Yes, it fixed file path and variable not existed in exhaustive search.
> Still see comments below.
>
> diff --git a/grub-core/osdep/windows/platform.c
> b/grub-core/osdep/windows/platform.c index b123256..3f4ad5e 100644
> --- a/grub-core/osdep/windows/platform.c
> +++ b/grub-core/osdep/windows/platform.c
> @@ -246,6 +246,8 @@ grub_install_register_efi (grub_device_t
> efidir_grub_dev, void *current = NULL;
> ssize_t current_len;
> current = get_efi_variable_bootn (i, ¤t_len);
> + if (current_len < 0)
> + continue; /* FIXME Should we abort on error? */
> if (current_len < (distrib16_len + 1) * sizeof
> (grub_uint16_t)
> + 6)
> {
>
> Same potential problem with signed vs. unsigned comparison. Variable is
> not guaranteed to exist here and -1 is treated as very large unsigned
> value.
>
> @@ -275,13 +277,18 @@ grub_install_register_efi (grub_device_t
> efidir_grub_dev, void *current = NULL;
> ssize_t current_len;
> current = get_efi_variable_bootn (i, ¤t_len);
> + if (current_len < -1)
> + continue; /* FIXME Should we abort on error? */
>
> For completeness we should handle failures to avoid crash.
>
> if (current_len == -1)
> {
> - order_num = i;
> - have_order_num = 1;
> - grub_util_info ("Creating new entry at Boot%04x",
> - order_num);
> - break;
> + if (!have_order_num)
> + {
> + order_num = i;
> + have_order_num = 1;
> + grub_util_info ("Creating new entry at Boot%04x",
> + order_num);
> + }
> + continue;
> }
>
> You changed it to stop on first non-existing variable; but this can
> miss another one later with the same description. This will result in
> two different boot entries with the same name.
>
> I cannot test it on Windows; on Linux code appears to be fast enough.
>
I've tested on windows (as applied in phcoder/windows64 branch). It's
fast enough as well. So go ahead for all 3 hunks.
>> As for merging with Linux part we could just have
>> primitives set variable and get variable and have everything else shared
>
> Yes, I'll get a look.
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
signature.asc
Description: OpenPGP digital signature