Re: [Qemu-devel] [PATCH 1/1] char/serial: Fix emptyness handling

[Don Slutz <dslutz@verizon.com>]

On 03/17/14 19:02, Peter Crosthwaite wrote:
> On Thu, Feb 20, 2014 at 4:30 AM, Don Slutz <dslutz@verizon.com> wrote:
>> The commit 88c1ee73d3231c74ff90bcfc084a7589670ec244
>> char/serial: Fix emptyness check
>>
>> Still causes extra NULL byte(s) to be sent.
>>
>> So if the fifo is empty, do not send an extra NULL byte.
>>
>> Signed-off-by: Don Slutz <dslutz@verizon.com>
>> ---
>>   hw/char/serial.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/char/serial.c b/hw/char/serial.c
>> index 6d3b5af..6df5a53 100644
>> --- a/hw/char/serial.c
>> +++ b/hw/char/serial.c
>> @@ -225,8 +225,9 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque)
>>
>>       if (s->tsr_retry <= 0) {
>>           if (s->fcr & UART_FCR_FE) {
>> -            s->tsr = fifo8_is_empty(&s->xmit_fifo) ?
>> -                        0 : fifo8_pop(&s->xmit_fifo);
>> +            if (fifo8_is_empty(&s->xmit_fifo))
>> +                return FALSE;
> If should have braces. The file in ancient and has many violations,
> but we are trying to update whenever we touch code as we go.
>
> Regards,
> Peter

Yes,  I have been working a lot on Xen and their coding style is the inverse and no checkpatch...

As expected this is reported:

WARNING: braces {} are necessary for all arms of this statement
#28: FILE: hw/char/serial.c:228:
+            if (fifo8_is_empty(&s->xmit_fifo))

v3 on its way.
     -Don Slutz

>> +            s->tsr = fifo8_pop(&s->xmit_fifo);
>>               if (!s->xmit_fifo.num) {
>>                   s->lsr |= UART_LSR_THRE;
>>               }
>> --
>> 1.8.4
>>
>>