On Wed, Mar 14, 2018 at 12:35:03PM +0000, Ard Biesheuvel wrote:
> >> I guess Leif and I are in disagreement here. In particular, I think
> >> his comment
> >>
> >> """
> >> ASSERT (FALSE)? (You already know Status is an EFI_ERROR, and a
> >> console message saying ASSERT (Status) is not getting you out of
> >> looking at the source code to find out what happened.)
> >> """
> >>
> >> is misguided, given that ASSERT_EFI_ERROR (Status) will actually print
> >> the value of Status to the debug console.
> >
> > I don't have a strong enough opinion on this that it should be
> > listened to if you both agree.
> >
> > It's just the "if error, then assert if error" that breaks up the
> > parsing flow for me. Could it make sense to use ASSERT_RETURN_ERROR
> > instead?
> >
> > So
> > if (EFI_ERROR (Status)) {
> > ASSERT_RETURN_ERROR (Status);
> > }
> >
>
> Do you mean using ASSERT_RETURN_ERROR() rather than ASSERT_EFI_ERROR()?
Yes.
> That doesn't make sense: the former is for RETURN_STATUS type
> variables and the latter for EFI_STATUS
Right, of course.
Makes the text look nicer though :)
Anyway, I'm OK with the original version.
Just wish we had a less redundant-looking way to describe this.
/
Leif
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel