.../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 110 ++++++++---------- 1 file changed, 50 insertions(+), 60 deletions(-)
Avoids having to use manual pointer manipulation.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
.../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 110 ++++++++----------
1 file changed, 50 insertions(+), 60 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index f71cc8188b4e..84d468ad3c8e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -713,18 +713,17 @@ static void bnxt_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
for (i = 0; i < bp->cp_nr_rings; i++) {
if (is_rx_ring(bp, i)) {
num_str = NUM_RING_RX_HW_STATS;
- for (j = 0; j < num_str; j++) {
- sprintf(buf, "[%d]: %s", i,
+ for (j = 0; j < num_str; j++)
+ ethtool_sprintf(
+ &buf, "[%d]: %s", i,
bnxt_ring_rx_stats_str[j]);
- buf += ETH_GSTRING_LEN;
- }
}
if (is_tx_ring(bp, i)) {
num_str = NUM_RING_TX_HW_STATS;
for (j = 0; j < num_str; j++) {
- sprintf(buf, "[%d]: %s", i,
+ ethtool_sprintf(
+ &buf, "[%d]: %s", i,
bnxt_ring_tx_stats_str[j]);
- buf += ETH_GSTRING_LEN;
}
}
num_str = bnxt_get_num_tpa_ring_stats(bp);
@@ -736,81 +735,72 @@ static void bnxt_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
else
str = bnxt_ring_tpa_stats_str;
- for (j = 0; j < num_str; j++) {
- sprintf(buf, "[%d]: %s", i, str[j]);
- buf += ETH_GSTRING_LEN;
- }
+ for (j = 0; j < num_str; j++)
+ ethtool_sprintf(&buf, "[%d]: %s", i, str[j]);
skip_tpa_stats:
if (is_rx_ring(bp, i)) {
num_str = NUM_RING_RX_SW_STATS;
- for (j = 0; j < num_str; j++) {
- sprintf(buf, "[%d]: %s", i,
+ for (j = 0; j < num_str; j++)
+ ethtool_sprintf(
+ &buf, "[%d]: %s", i,
bnxt_rx_sw_stats_str[j]);
- buf += ETH_GSTRING_LEN;
- }
}
num_str = NUM_RING_CMN_SW_STATS;
- for (j = 0; j < num_str; j++) {
- sprintf(buf, "[%d]: %s", i,
- bnxt_cmn_sw_stats_str[j]);
- buf += ETH_GSTRING_LEN;
- }
- }
- for (i = 0; i < BNXT_NUM_RING_ERR_STATS; i++) {
- strscpy(buf, bnxt_ring_err_stats_arr[i], ETH_GSTRING_LEN);
- buf += ETH_GSTRING_LEN;
+ for (j = 0; j < num_str; j++)
+ ethtool_sprintf(&buf, "[%d]: %s", i,
+ bnxt_cmn_sw_stats_str[j]);
}
+ for (i = 0; i < BNXT_NUM_RING_ERR_STATS; i++)
+ ethtool_puts(&buf, bnxt_ring_err_stats_arr[i]);
+
+ if (bp->flags & BNXT_FLAG_PORT_STATS)
+ for (i = 0; i < BNXT_NUM_PORT_STATS; i++)
+ ethtool_puts(&buf,
+ bnxt_port_stats_arr[i].string);
- if (bp->flags & BNXT_FLAG_PORT_STATS) {
- for (i = 0; i < BNXT_NUM_PORT_STATS; i++) {
- strcpy(buf, bnxt_port_stats_arr[i].string);
- buf += ETH_GSTRING_LEN;
- }
- }
if (bp->flags & BNXT_FLAG_PORT_STATS_EXT) {
u32 len;
len = min_t(u32, bp->fw_rx_stats_ext_size,
ARRAY_SIZE(bnxt_port_stats_ext_arr));
- for (i = 0; i < len; i++) {
- strcpy(buf, bnxt_port_stats_ext_arr[i].string);
- buf += ETH_GSTRING_LEN;
- }
+ for (i = 0; i < len; i++)
+ ethtool_puts(&buf,
+ bnxt_port_stats_ext_arr[i].string);
+
len = min_t(u32, bp->fw_tx_stats_ext_size,
ARRAY_SIZE(bnxt_tx_port_stats_ext_arr));
- for (i = 0; i < len; i++) {
- strcpy(buf,
- bnxt_tx_port_stats_ext_arr[i].string);
- buf += ETH_GSTRING_LEN;
- }
+ for (i = 0; i < len; i++)
+ ethtool_puts(
+ &buf,
+ bnxt_tx_port_stats_ext_arr[i].string);
+
if (bp->pri2cos_valid) {
- for (i = 0; i < 8; i++) {
- strcpy(buf,
- bnxt_rx_bytes_pri_arr[i].string);
- buf += ETH_GSTRING_LEN;
- }
- for (i = 0; i < 8; i++) {
- strcpy(buf,
- bnxt_rx_pkts_pri_arr[i].string);
- buf += ETH_GSTRING_LEN;
- }
- for (i = 0; i < 8; i++) {
- strcpy(buf,
- bnxt_tx_bytes_pri_arr[i].string);
- buf += ETH_GSTRING_LEN;
- }
- for (i = 0; i < 8; i++) {
- strcpy(buf,
- bnxt_tx_pkts_pri_arr[i].string);
- buf += ETH_GSTRING_LEN;
- }
+ for (i = 0; i < 8; i++)
+ ethtool_puts(
+ &buf,
+ bnxt_rx_bytes_pri_arr[i].string);
+
+ for (i = 0; i < 8; i++)
+ ethtool_puts(
+ &buf,
+ bnxt_rx_pkts_pri_arr[i].string);
+
+ for (i = 0; i < 8; i++)
+ ethtool_puts(
+ &buf,
+ bnxt_tx_bytes_pri_arr[i].string);
+
+ for (i = 0; i < 8; i++)
+ ethtool_puts(
+ &buf,
+ bnxt_tx_pkts_pri_arr[i].string);
}
}
break;
case ETH_SS_TEST:
if (bp->num_tests)
- memcpy(buf, bp->test_info->string,
- bp->num_tests * ETH_GSTRING_LEN);
+ for (i = 0; i < bp->num_tests; i++)
+ ethtool_puts(&buf, bp->test_info->string[i]);
break;
default:
netdev_err(bp->dev, "bnxt_get_strings invalid request %x\n",
--
2.47.0On Sun, Oct 20, 2024 at 7:29 PM Rosen Penev <rosenp@gmail.com> wrote:
>
> Avoids having to use manual pointer manipulation.
>
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
> .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 110 ++++++++----------
> 1 file changed, 50 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> index f71cc8188b4e..84d468ad3c8e 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> @@ -713,18 +713,17 @@ static void bnxt_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
> for (i = 0; i < bp->cp_nr_rings; i++) {
> if (is_rx_ring(bp, i)) {
> num_str = NUM_RING_RX_HW_STATS;
> - for (j = 0; j < num_str; j++) {
> - sprintf(buf, "[%d]: %s", i,
> + for (j = 0; j < num_str; j++)
> + ethtool_sprintf(
> + &buf, "[%d]: %s", i,
If you combine this line with the line above, I think it will look better:
ethtool_sprintf(&buf, "[%d]: %s", i,
checkpatch.pl may warn because the next line won't line up, but I
think it will look better and it reduces the number of lines. The
same comment applies to other similar spots in the patch. Thanks.
> bnxt_ring_rx_stats_str[j]);
> - buf += ETH_GSTRING_LEN;
> - }
On Mon, Oct 21, 2024 at 1:26 PM Michael Chan <michael.chan@broadcom.com> wrote:
>
> On Sun, Oct 20, 2024 at 7:29 PM Rosen Penev <rosenp@gmail.com> wrote:
> >
> > Avoids having to use manual pointer manipulation.
> >
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> > .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 110 ++++++++----------
> > 1 file changed, 50 insertions(+), 60 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> > index f71cc8188b4e..84d468ad3c8e 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> > @@ -713,18 +713,17 @@ static void bnxt_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
> > for (i = 0; i < bp->cp_nr_rings; i++) {
> > if (is_rx_ring(bp, i)) {
> > num_str = NUM_RING_RX_HW_STATS;
> > - for (j = 0; j < num_str; j++) {
> > - sprintf(buf, "[%d]: %s", i,
> > + for (j = 0; j < num_str; j++)
> > + ethtool_sprintf(
> > + &buf, "[%d]: %s", i,
>
> If you combine this line with the line above, I think it will look better:
>
> ethtool_sprintf(&buf, "[%d]: %s", i,
>
> checkpatch.pl may warn because the next line won't line up, but I
> think it will look better and it reduces the number of lines. The
> same comment applies to other similar spots in the patch. Thanks.
I use git-clang-format for this stuff.
The only real way to satisfy everything is to use a temporary variable like
for (i = 0; i < BNXT_NUM_PORT_STATS; i++) {
const char *str = bnxt_port_stats_arr[i].string;
ethtool_puts(&buf, str);
}
to make everything small.
>
> > bnxt_ring_rx_stats_str[j]);
> > - buf += ETH_GSTRING_LEN;
> > - }
On Mon, Oct 21, 2024 at 2:05 PM Rosen Penev <rosenp@gmail.com> wrote:
> The only real way to satisfy everything is to use a temporary variable like
>
> for (i = 0; i < BNXT_NUM_PORT_STATS; i++) {
> const char *str = bnxt_port_stats_arr[i].string;
>
> ethtool_puts(&buf, str);
> }
>
> to make everything small.
This looks fine too. The actual function call looks clean on one line.
On Sun, Oct 20, 2024 at 07:29:01PM -0700, Rosen Penev wrote: > Avoids having to use manual pointer manipulation. > > Signed-off-by: Rosen Penev <rosenp@gmail.com> Reviewed-by: Simon Horman <horms@kernel.org>
© 2016 - 2024 Red Hat, Inc.