Philippe Mathieu-Daudé <phi...@redhat.com> writes:

> On 2/19/19 2:41 PM, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <phi...@redhat.com> writes:
>> 
>>> On 2/18/19 1:56 PM, Markus Armbruster wrote:
>>>> flash.h's incomplete struct pflash_t is completed both in
>>>> pflash_cfi01.c and in pflash_cfi02.c.  The complete types are
>>>> incompatible.  This can hide type errors, such as passing a pflash_t
>>>> created with pflash_cfi02_register() to pflash_cfi01_get_memory().
>>>>
>>>> Furthermore, POSIX reserves typedef names ending with _t.
>
> Worth adding in CODING_STYLE 'Naming' section :)
>
>>>>
>>>> Rename the two structs to PFlashCFI01 and PFlashCFI02.
>>>
>>> Why not ParallelFlashCFIxx?
>> 
>> Feels a bit long, and we abbreviate to pflash pretty consistently.  That
>> said, I'm not particularly enamored with my choice of name :)
>> 
>>> Ideally ParallelFlashCFI would be an InterfaceInfo...
>> 
>> You mean TYPE_CFI_PFLASH0{1,2} should be children of an abstract parent?
>
> I'd use "TYPE_PFLASH_CFI0[12]".

That's a separate renaming patch.  It could go right before PATCH 03.
Worthwhile?

[...]

Reply via email to