On 05/12/2017 at 21:18, Julia Cartwright wrote:
> The list_for_each_entry() macro already handles the case where the list
> is empty (by not executing the loop body).  It's not necessary to handle
> this case specially, so stop doing so.
> 
> Cc: Rafal Ozieblo <raf...@cadence.com>
> Signed-off-by: Julia Cartwright <ju...@ni.com>
> ---
> This is an additional cleanup patch found when looking at this code.
> 
>    Julia

Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>

Thanks

> 
>  drivers/net/ethernet/cadence/macb_main.c | 34 
> ++++++++++++--------------------
>  1 file changed, 13 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c 
> b/drivers/net/ethernet/cadence/macb_main.c
> index e7ef104a077d..3643c6ad2322 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -2815,25 +2815,22 @@ static int gem_add_flow_filter(struct net_device 
> *netdev,
>       spin_lock_irqsave(&bp->rx_fs_lock, flags);
>  
>       /* find correct place to add in list */
> -     if (list_empty(&bp->rx_fs_list.list))
> -             list_add(&newfs->list, &bp->rx_fs_list.list);
> -     else {
> -             list_for_each_entry(item, &bp->rx_fs_list.list, list) {
> -                     if (item->fs.location > newfs->fs.location) {
> -                             list_add_tail(&newfs->list, &item->list);
> -                             added = true;
> -                             break;
> -                     } else if (item->fs.location == fs->location) {
> -                             netdev_err(netdev, "Rule not added: location %d 
> not free!\n",
> -                                             fs->location);
> -                             ret = -EBUSY;
> -                             goto err;
> -                     }
> +     list_for_each_entry(item, &bp->rx_fs_list.list, list) {
> +             if (item->fs.location > newfs->fs.location) {
> +                     list_add_tail(&newfs->list, &item->list);
> +                     added = true;
> +                     break;
> +             } else if (item->fs.location == fs->location) {
> +                     netdev_err(netdev, "Rule not added: location %d not 
> free!\n",
> +                                     fs->location);
> +                     ret = -EBUSY;
> +                     goto err;
>               }
> -             if (!added)
> -                     list_add_tail(&newfs->list, &bp->rx_fs_list.list);
>       }
>  
> +     if (!added)
> +             list_add_tail(&newfs->list, &bp->rx_fs_list.list);
> +
>       gem_prog_cmp_regs(bp, fs);
>       bp->rx_fs_list.count++;
>       /* enable filtering if NTUPLE on */
> @@ -2859,11 +2856,6 @@ static int gem_del_flow_filter(struct net_device 
> *netdev,
>  
>       spin_lock_irqsave(&bp->rx_fs_lock, flags);
>  
> -     if (list_empty(&bp->rx_fs_list.list)) {
> -             spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
> -             return -EINVAL;
> -     }
> -
>       list_for_each_entry(item, &bp->rx_fs_list.list, list) {
>               if (item->fs.location == cmd->fs.location) {
>                       /* disable screener regs for the flow entry */
> 


-- 
Nicolas Ferre

Reply via email to