On 05/08/2018 03:36 PM, Andrew Rybchenko wrote:
Many thanks. I guess the most of below notes are applicable to many other
patches in the series.

Signed-off-by: ,  Fixes: and Cc: sta...@dpdk.org tags are missing. See [1].

Everybody's project has different prejudices.

Changeset summary should start from "net/sfc: "
I.e. something like:
net/sfc: fix strncpy size and NUL

Yeah if that's what you like.

(it looks like "another" is useless in the original subject)

It captures my feeling at having to wade through making 18 fixes before I could compile the project on current Fedora.

In general all patches should pass ./devtools/check-git-log.sh and ./devtools/checkpatches.sh
(which requires path to Linux kernel checkpatches.pl).

Can you help me understand why adding CRLFs at 80 cols on the gcc errors I pasted helps anything at all? The patches actually fix problems in the code.

If you don't care about Coverity, let me know and I will register this project there and send you fixes when I have time.

Andrew.

[1] http://dpdk.org/doc/guides/contributing/patches.html#commit-messages-subject-line

On 05/08/2018 07:30 AM, Andy Green wrote:
---
  drivers/net/sfc/sfc_ethdev.c |    7 +++++--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index e9bb283e0..bd5f17f33 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -662,10 +662,13 @@ sfc_xstats_get_names(struct rte_eth_dev *dev,
for (i = 0; i < EFX_MAC_NSTATS; ++i) {
                if (EFX_MAC_STAT_SUPPORTED(port->mac_stats_mask, i)) {
-                       if (xstats_names != NULL && nstats < xstats_count)
+                       if (xstats_names != NULL && nstats < xstats_count) {
                                strncpy(xstats_names[nstats].name,
                                        efx_mac_stat_name(sa->nic, i),
-                                       sizeof(xstats_names[0].name));
+                                       sizeof(xstats_names[0].name) - 1);
+                               xstats_names[0].name[
+                                       sizeof(xstats_names[0].name) - 1] = 
'\0';
+                       }

In fact strlcpy() should be used.
Fair enough.  Last time I looked it wasn't in glibc but seems it is now.

-Andy

                        nstats++;
                }
        }


Reply via email to