On Wed, Oct 4, 2017 at 11:25 AM, Alexander Graf <ag...@suse.de> wrote: > > > On 04.10.17 16:57, Rob Clark wrote: >> >> On Wed, Oct 4, 2017 at 10:46 AM, Heinrich Schuchardt <xypron.g...@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.g...@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 <robdcl...@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. > > > I don't have a concrete plan - in general I consider patches that have > unaddressed review comments as "not to be applied atm" though and I'm not > sure I have anything pending from you that would not fall into that category > :). > > Can you split off a series that has Heinrich's blessing to get us as far as > we can, so we can keep your queue short? > >> >>> 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. > > > Feel free to implement just the boilerplate for it with a function that > always returns true: > > static int efi_ptr_valid(void *foo) > { > return 1; > }
that is reasonable.. I do think we want something to identify types (which could just be a unique global/const ptr for now), so like: static inline bool efi_ptr_valid(void *ptr, efi_type_t *type) (which would let us change what a type is later if we needed) I'll try to cook something up (which might not be until the weekend.. still getting caught up after two weeks of conferences) BR, -R > which we can then later on improve to do actual checking if we care. The > least we can do is probably to check for alignment problems. > > > Alex _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
