| Message ID | 14024ddeb2b3a8c5b0138b5ba5083f54d00164a9.1619594713.git.christophe.jaillet@wanadoo.fr |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] serial: 8250: Use 'hlist_for_each_entry' to simplify code | expand |
On Wed, Apr 28, 2021 at 09:30:40AM +0200, Christophe JAILLET wrote: > Use 'hlist_for_each_entry' instead of hand writing it. > This saves a few lines of code. > > The comment about warning generated by some gcc version is also removed. > The way 'hlist_for_each_entry' is written should prevent such a warning to > be emitted. I checked the implementation of the macros vs. this usage and below seems legit. However it indeed needs more test coverage because it's sensitive piece of code. FWIW, Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> (with the above caveat) > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > Compile tested only > --- > drivers/tty/serial/8250/8250_core.c | 18 ++++-------------- > 1 file changed, 4 insertions(+), 14 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c > index cae61d1ebec5..081b773a54c9 100644 > --- a/drivers/tty/serial/8250/8250_core.c > +++ b/drivers/tty/serial/8250/8250_core.c > @@ -172,7 +172,6 @@ static void serial_do_unlink(struct irq_info *i, struct uart_8250_port *up) > static int serial_link_irq_chain(struct uart_8250_port *up) > { > struct hlist_head *h; > - struct hlist_node *n; > struct irq_info *i; > int ret; > > @@ -180,13 +179,11 @@ static int serial_link_irq_chain(struct uart_8250_port *up) > > h = &irq_lists[up->port.irq % NR_IRQ_HASH]; > > - hlist_for_each(n, h) { > - i = hlist_entry(n, struct irq_info, node); > + hlist_for_each_entry(i, h, node) > if (i->irq == up->port.irq) > break; > - } > > - if (n == NULL) { > + if (i == NULL) { > i = kzalloc(sizeof(struct irq_info), GFP_KERNEL); > if (i == NULL) { > mutex_unlock(&hash_mutex); > @@ -220,25 +217,18 @@ static int serial_link_irq_chain(struct uart_8250_port *up) > > static void serial_unlink_irq_chain(struct uart_8250_port *up) > { > - /* > - * yes, some broken gcc emit "warning: 'i' may be used uninitialized" > - * but no, we are not going to take a patch that assigns NULL below. > - */ > struct irq_info *i; > - struct hlist_node *n; > struct hlist_head *h; > > mutex_lock(&hash_mutex); > > h = &irq_lists[up->port.irq % NR_IRQ_HASH]; > > - hlist_for_each(n, h) { > - i = hlist_entry(n, struct irq_info, node); > + hlist_for_each_entry(i, h, node) > if (i->irq == up->port.irq) > break; > - } > > - BUG_ON(n == NULL); > + BUG_ON(i == NULL); > BUG_ON(i->head == NULL); > > if (list_empty(i->head)) > -- > 2.30.2 >
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index cae61d1ebec5..081b773a54c9 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -172,7 +172,6 @@ static void serial_do_unlink(struct irq_info *i, struct uart_8250_port *up) static int serial_link_irq_chain(struct uart_8250_port *up) { struct hlist_head *h; - struct hlist_node *n; struct irq_info *i; int ret; @@ -180,13 +179,11 @@ static int serial_link_irq_chain(struct uart_8250_port *up) h = &irq_lists[up->port.irq % NR_IRQ_HASH]; - hlist_for_each(n, h) { - i = hlist_entry(n, struct irq_info, node); + hlist_for_each_entry(i, h, node) if (i->irq == up->port.irq) break; - } - if (n == NULL) { + if (i == NULL) { i = kzalloc(sizeof(struct irq_info), GFP_KERNEL); if (i == NULL) { mutex_unlock(&hash_mutex); @@ -220,25 +217,18 @@ static int serial_link_irq_chain(struct uart_8250_port *up) static void serial_unlink_irq_chain(struct uart_8250_port *up) { - /* - * yes, some broken gcc emit "warning: 'i' may be used uninitialized" - * but no, we are not going to take a patch that assigns NULL below. - */ struct irq_info *i; - struct hlist_node *n; struct hlist_head *h; mutex_lock(&hash_mutex); h = &irq_lists[up->port.irq % NR_IRQ_HASH]; - hlist_for_each(n, h) { - i = hlist_entry(n, struct irq_info, node); + hlist_for_each_entry(i, h, node) if (i->irq == up->port.irq) break; - } - BUG_ON(n == NULL); + BUG_ON(i == NULL); BUG_ON(i->head == NULL); if (list_empty(i->head))
Use 'hlist_for_each_entry' instead of hand writing it. This saves a few lines of code. The comment about warning generated by some gcc version is also removed. The way 'hlist_for_each_entry' is written should prevent such a warning to be emitted. Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- Compile tested only --- drivers/tty/serial/8250/8250_core.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-)