[U-Boot] [PATCH v4 2/2] efi_selftest: provide a test for block io

[Simon Glass]

Hi Heinrich,

On 4 February 2018 at 15:35, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> On 02/04/2018 01:39 PM, Simon Glass wrote:
>> On 21 January 2018 at 11:29, Heinrich Schuchardt <xypron.glpk at gmx.de>
wrote:
>>> This test checks the driver for block IO devices.
>>> A disk image is created in memory.
>>> A handle is created for the new block IO device.
>>> The block I/O protocol is installed on the handle.
>>> ConnectController is used to setup partitions and to install the simple
>>> file protocol.
>>> A known file is read from the file system and verified.
>>>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>> ---
>>> v4
>>>         Provide a commit message.
>>> v3
>>>         no change
>>> v2
>>>         no change
>>> ---
>>>  lib/efi_selftest/Makefile                    |   3 +
>>>  lib/efi_selftest/efi_selftest_block_device.c | 395
+++++++++++++++++++++++++++
>>>  lib/efi_selftest/efi_selftest_disk_image.h   |  69 +++++
>>>  3 files changed, 467 insertions(+)
>>>  create mode 100644 lib/efi_selftest/efi_selftest_block_device.c
>>>  create mode 100644 lib/efi_selftest/efi_selftest_disk_image.h
>>
>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>
>> A few general comments:
>>
>> - I'm not very keen on using 'this' in the code. This is not C++. Can
>> you use blkio or something, instead?
>
> The UEFI spec uses the variable name 'this' in the definition of the
> EFI_DRIVER_BINDING_PROTOCOL and as already discussed I prefer to stick
> to the commonly known variable names.

I don't see why the EFI spec has to bleed into its code implementation. We
are talking about a parameter name, not a method or struct name.

>
> 'this' refers to the EFI_DRIVER_BINDING_PROTOCOL object itself and not
> to "blkio or something".
>
>> - How about declaring all the static vars at the top of the file?
>
> Did you mean put all struct definitions above all static vars?
> Putting vars above the definition of their types wouldn't make sense.

Yes I think decls should normally got at the top of a file so they are easy
to find.

Regards,
Simon