[U-Boot] [PATCH 1/1] efi_loader: use type bool for event states

[Rob Clark]

On Wed, Oct 4, 2017 at 10:46 AM, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> On 10/04/2017 04:14 PM, Rob Clark wrote:
>> On Wed, Oct 4, 2017 at 9:03 AM, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>> Queued and signaled describe boolean states of events.
>>> So let's use type bool and rename the structure members to is_queued
>>> and is_signaled.
>>>
>>> Update the comments for is_queued and is_signaled.
>>
>> Reviewed-by: Rob Clark <robdclark at gmail.com>
>>
>> It would be kinda nice to merge my efi_event fixes and rework to use
>> an arbitrary sized list of events before making too many more
>> efi_event changes, since that is kind of annoying to keep rebasing ;-)
>>
>> BR,
>> -R
>
> I would not mind if you patch went first.
>
> But your patch
> https://patchwork.ozlabs.org/patch/812967/
> is not applicable to U-Boot master and needs rebasing anyway.

jfyi, I have it (and other pending patches) rebased on latest master
(as of ~yesterday) here:

  https://github.com/robclark/u-boot/commits/wip-enough-uefi-for-shell

I wasn't planning on resending until I get further with FAT write
stuff (currently on a local branch, although I might not get much time
to work on in the next week or two).. although I can re-send it or any
of the other patches to get Shell.efi working if wanted.  (Note that
I'm also using your patch for efi watchdog support, that was one of
the other required bits.)

Not sure what agraf's plan is but I think the needed bits for
Shell.efi are mergable already.

> Please, add the missing check that the event pointer is valid.
> The EFI code checks other arguments rigorously so we should do the same
> for pointers. It would be very hard to debug a problem in an EFI
> application otherwise.

I'm a bit undecided on this, since we have other places where there is
no good way to check the validity of a pointer.  (For example file or
disk objects.)  I was thinking about perhaps implementing a
compile-time optional feature using a hashtable to map objects to type
so we can add in some type checking, at the expense of extra runtime
overhead.  Probably not something you'd want to ship enabled, but it
would be useful for debugging.

BR,
-R

> @Alexander
> I guess with Suseconf you were quite busy in the last weeks. Did you
> already make up your mind in which sequence we should prepare the EFI
> patches?
>
> The following of my patches could be directly applied to efi-next:
>
> [U-Boot,1/1] efi_selftest: enable CONFIG_CMD_BOOTEFI_SELFTEST
> https://patchwork.ozlabs.org/patch/816412/
> [U-Boot,1/1] efi_loader: replace efi_div10 by div64_u64
> https://patchwork.ozlabs.org/patch/820731/
> [U-Boot,1/1] efi_selftest: use efi_st_error for all error messages
> https://patchwork.ozlabs.org/patch/821237/
> [U-Boot,1/1] efi_loader: use type bool for event states
> https://patchwork.ozlabs.org/patch/821302/
> [U-Boot,v2,1/1] efi_selftest: make tests easier to read
> https://patchwork.ozlabs.org/patch/821306/
>
> I guess Rob was refering to
> [U-Boot,1/1] efi_loader: use type bool for event states
>
> Best regards
>
> Heinrich