Re: [Qemu-devel] [PATCH 1/7] serial: add serial_chr_nonnull() to use the null backend when none provided

[Markus Armbruster]

Philippe Mathieu-Daudé <address@hidden> writes:

> On 08/31/2017 06:43 AM, Thomas Huth wrote:
>> On 31.08.2017 11:36, Marc-André Lureau wrote:
>>> Hi
>>>
>>> On Thu, Aug 31, 2017 at 7:20 AM Thomas Huth <address@hidden
>>> <mailto:address@hidden>> wrote:
>>>
>>>      On 31.08.2017 05:53, Philippe Mathieu-Daudé wrote:
>>>      > Suggested-by: Peter Maydell <address@hidden
>>>      <mailto:address@hidden>>
>>>      > Signed-off-by: Philippe Mathieu-Daudé <address@hidden
>>>      <mailto:address@hidden>>
>>>      > ---
>>>      >  include/hw/char/serial.h |  1 +
>>>      >  hw/char/serial.c         | 13 +++++++++++++
>>>      >  2 files changed, 14 insertions(+)
>>>      >
>>>      > diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
>>>      > index c4daf11a14..96bccb39e1 100644
>>>      > --- a/include/hw/char/serial.h
>>>      > +++ b/include/hw/char/serial.h
>>>      > @@ -93,6 +93,7 @@ SerialState *serial_mm_init(MemoryRegion
>>>      *address_space,
>>>      >                              hwaddr base, int it_shift,
>>>      >                              qemu_irq irq, int baudbase,
>>>      >                              Chardev *chr, enum device_endian end);
>>>      > +Chardev *serial_chr_nonnull(Chardev *chr);
>>>
>>>      Why do you need the prototype? Please make the function static if
>>>      possible (otherwise please add some rationale in the patch 
>>> description).
>>>
>>> It's being used from other units in following patches
>>
>> Ah, well, right. I was only on CC: in the first two patches, so I missed
>> the other ones at the first glance. So never mind my comment, the
>> prototype is fine here.
>
> Is it better/easier to use the same list for the cover and all the patches?
> I try to shorten the it to help overloaded reviewer to focus on the
> patches I think they can help. But this case show it's not that
> useful.

Clear an unequivocal answer: it depends :)