Re: [PATCH 1/3] can: proc: make array printing function indenpendent from sff frames

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 3607 bytes --]

On 04/01/2014 08:57 PM, Oliver Hartkopp wrote:
> The can_rcvlist_sff_proc_show_one() function which prints the array of filters
> for the single SFF CAN identifiers is prepared to be used by a second caller.
> Therefore it is also renamed to properly describe its future functionality.
> 
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>

Looks good, some nitpicks inline.

> ---
>  net/can/af_can.h |  4 +++-
>  net/can/proc.c   | 28 ++++++++++++++++------------
>  2 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/net/can/af_can.h b/net/can/af_can.h
> index 6de58b4..989e695 100644
> --- a/net/can/af_can.h
> +++ b/net/can/af_can.h
> @@ -59,12 +59,14 @@ struct receiver {
>  	char *ident;
>  };
>  
> +#define CAN_SFF_RCV_ARRAY_SZ (1<<CAN_SFF_ID_BITS)

Please add spaces around the <<

> +
>  enum { RX_ERR, RX_ALL, RX_FIL, RX_INV, RX_EFF, RX_MAX };
>  
>  /* per device receive filters linked at dev->ml_priv */
>  struct dev_rcv_lists {
>  	struct hlist_head rx[RX_MAX];
> -	struct hlist_head rx_sff[0x800];
> +	struct hlist_head rx_sff[CAN_SFF_RCV_ARRAY_SZ];
>  	int remove_on_zero_entries;
>  	int entries;
>  };
> diff --git a/net/can/proc.c b/net/can/proc.c
> index b543470..1f8e1e6 100644
> --- a/net/can/proc.c
> +++ b/net/can/proc.c
> @@ -389,25 +389,26 @@ static const struct file_operations can_rcvlist_proc_fops = {
>  	.release	= single_release,
>  };
>  
> -static inline void can_rcvlist_sff_proc_show_one(struct seq_file *m,
> -						 struct net_device *dev,
> -						 struct dev_rcv_lists *d)
> +static inline void can_rcvlist_proc_show_array(struct seq_file *m,
> +					       struct net_device *dev,
> +					       struct hlist_head *rcv_array,
> +					       unsigned int rcv_array_sz)
>  {
	> -	int i;
> +	unsigned int i;
>  	int all_empty = 1;
>  
>  	/* check whether at least one list is non-empty */
> -	for (i = 0; i < 0x800; i++)
> -		if (!hlist_empty(&d->rx_sff[i])) {
> +	for (i = 0; i < rcv_array_sz; i++)
> +		if (!hlist_empty(&rcv_array[i])) {
>  			all_empty = 0;
>  			break;
>  		}
>  
>  	if (!all_empty) {
>  		can_print_recv_banner(m);
> -		for (i = 0; i < 0x800; i++) {
> -			if (!hlist_empty(&d->rx_sff[i]))
> -				can_print_rcvlist(m, &d->rx_sff[i], dev);
> +		for (i = 0; i < rcv_array_sz; i++) {
> +			if (!hlist_empty(&rcv_array[i]))
> +				can_print_rcvlist(m, &rcv_array[i], dev);
>  		}
>  	} else
>  		seq_printf(m, "  (%s: no entry)\n", DNAME(dev));
> @@ -425,12 +426,15 @@ static int can_rcvlist_sff_proc_show(struct seq_file *m, void *v)
>  
>  	/* sff receive list for 'all' CAN devices (dev == NULL) */
>  	d = &can_rx_alldev_list;
> -	can_rcvlist_sff_proc_show_one(m, NULL, d);
> +	can_rcvlist_proc_show_array(m, NULL, d->rx_sff, CAN_SFF_RCV_ARRAY_SZ);

Please use ARRAY_SIZE(d->rx_sff)

>  
>  	/* sff receive list for registered CAN devices */
>  	for_each_netdev_rcu(&init_net, dev) {
> -		if (dev->type == ARPHRD_CAN && dev->ml_priv)
> -			can_rcvlist_sff_proc_show_one(m, dev, dev->ml_priv);
> +		if (dev->type == ARPHRD_CAN && dev->ml_priv) {
> +			d = dev->ml_priv;
> +			can_rcvlist_proc_show_array(m, dev, d->rx_sff,
> +						    CAN_SFF_RCV_ARRAY_SZ);
Please use ARRAY_SIZE(d->rx_sff)
> +		}
>  	}
>  
>  	rcu_read_unlock();
> 
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]