[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] upgrading emulated UART to 16550A
From: |
Stefano Stabellini |
Subject: |
Re: [Qemu-devel] [PATCH] upgrading emulated UART to 16550A |
Date: |
Fri, 08 Aug 2008 15:36:50 +0100 |
User-agent: |
Thunderbird 2.0.0.14 (X11/20080505) |
Anthony Liguori wrote:
>
> I think the idea was to add a new copyright entry, not to modify
> Fabrice's copyright.
Oops, I am going to fix that.
>> +
>> +#include <termios.h>
>> +#include <sys/ioctl.h>
>>
>
> This doesn't look Windows friendly.
I am going to follow Samuel advice and declare more constants in
qemu-char.h so that we don't need those two headers anymore.
>
> minor nit, but the '{' should be on a new line.
>
...
> More nits, lose the whitespace in the conditions. For instance:
>
> } else if ( ( s->ier & UART_IER_RDI ) => } else if ((s->ier &
> UART_IER_RDI).
>
> The rest of the file uses the later style so it's a little weird to have
> portions of the code be different.
>
...
>
> More bad whitespace.
>
...
>
> This is just nutty :-) Please rewrite this if() statement to be a
> little less obscure.
>
All these style problems will be fixed in the next version.
>> + serial_init_core(s, irq, baudbase, chr);
>> register_savevm("serial", base, 2, serial_save, serial_load, s);
>>
>
> Isn't it necessary to bump the savevm() version number since you've
> changed the format.
>
Yes, you are right.
Thanks for this very helpful review.
[Qemu-devel] [PATCH] upgrading emulated UART to 16550A, Stefano Stabellini, 2008/08/08
[Qemu-devel] [PATCH] upgrading emulated UART to 16550A, Stefano Stabellini, 2008/08/08