* Paolo Bonzini (pbonz...@redhat.com) wrote:
> It can never become negative; reflect this in the type of the field
> and simplify the conditions.
OK, yes, the one case that could make it go -ve was removed in fcfb4d6
somewhere after 1.4.0.
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> hw/char/serial.c | 12 ++++++++----
> include/hw/char/serial.h | 2 +-
> 2 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index 6d815b5..e65e9e0 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -229,7 +229,7 @@ static gboolean serial_xmit(GIOChannel *chan,
> GIOCondition cond, void *opaque)
>
> do {
> assert(!(s->lsr & UART_LSR_TEMT));
> - if (s->tsr_retry <= 0) {
> + if (s->tsr_retry == 0) {
> assert(!(s->lsr & UART_LSR_THRE));
>
> if (s->fcr & UART_FCR_FE) {
> @@ -252,7 +252,7 @@ static gboolean serial_xmit(GIOChannel *chan,
> GIOCondition cond, void *opaque)
> /* in loopback mode, say that we just received a char */
> serial_receive1(s, &s->tsr, 1);
> } else if (qemu_chr_fe_write(s->chr, &s->tsr, 1) != 1) {
> - if (s->tsr_retry >= 0 && s->tsr_retry < MAX_XMIT_RETRY &&
> + if (s->tsr_retry < MAX_XMIT_RETRY &&
> qemu_chr_fe_add_watch(s->chr, G_IO_OUT|G_IO_HUP,
> serial_xmit, s) > 0) {
> s->tsr_retry++;
> @@ -330,7 +330,7 @@ static void serial_ioport_write(void *opaque, hwaddr
> addr, uint64_t val,
> s->lsr &= ~UART_LSR_THRE;
> s->lsr &= ~UART_LSR_TEMT;
> serial_update_irq(s);
> - if (s->tsr_retry <= 0) {
> + if (s->tsr_retry == 0) {
> serial_xmit(NULL, G_IO_OUT, s);
> }
> }
> @@ -639,6 +639,10 @@ static int serial_post_load(void *opaque, int version_id)
> if (s->thr_ipending == -1) {
> s->thr_ipending = ((s->iir & UART_IIR_ID) == UART_IIR_THRI);
> }
> + if (s->tsr_retry > MAX_XMIT_RETRY) {
> + s->tsr_retry = MAX_XMIT_RETRY;
> + }
Good that makes it safe even after receiving the case that could be -1.
Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> s->last_break_enable = (s->lcr >> 6) & 1;
> /* Initialize fcr via setter to perform essential side-effects */
> serial_write_fcr(s, s->fcr_vmstate);
> @@ -685,7 +689,7 @@ static const VMStateDescription vmstate_serial_tsr = {
> .minimum_version_id = 1,
> .needed = serial_tsr_needed,
> .fields = (VMStateField[]) {
> - VMSTATE_INT32(tsr_retry, SerialState),
> + VMSTATE_UINT32(tsr_retry, SerialState),
> VMSTATE_UINT8(thr, SerialState),
> VMSTATE_UINT8(tsr, SerialState),
> VMSTATE_END_OF_LIST()
> diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
> index 15beb6b..ef90615 100644
> --- a/include/hw/char/serial.h
> +++ b/include/hw/char/serial.h
> @@ -55,7 +55,7 @@ struct SerialState {
> int last_break_enable;
> int it_shift;
> int baudbase;
> - int tsr_retry;
> + unsigned tsr_retry;
> uint32_t wakeup;
>
> /* Time when the last byte was successfully sent out of the tsr */
> --
> 2.5.5
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
