Re: [PATCH v2 1/2] net/mlx4: split the definitions to the header file

Hi Ferruh,

-----Original Message-----
From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
Sent: Wednesday, February 22, 2017 21:05
To: Vasily Philipov <redacted>; dev@dpdk.org
Cc: Adrien Mazarguil <redacted>; Nélio Laranjeiro
[off-list ref]
Subject: Re: [dpdk-dev] [PATCH v2 1/2] net/mlx4: split the definitions to the
header file

On 2/22/2017 1:42 PM, Vasily Philipov wrote:

quoted

Make some structs/defines visible from different source files by
placing them into mlx4.h header.

Signed-off-by: Vasily Philipov <redacted>
---
 drivers/net/mlx4/mlx4.c | 183
++--------------------------------------------
 drivers/net/mlx4/mlx4.h | 187
+++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 189 insertions(+), 181 deletions(-)

diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c index

79efaaa..82ccac8 100644

--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c

@@ -1,8 +1,8 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright 2012-2015 6WIND S.A.
- *   Copyright 2012 Mellanox.
+ *   Copyright 2012-2017 6WIND S.A.
+ *   Copyright 2012-2017 Mellanox.

Can someone knowledgeable about Copyright help please?

What is the year field in Copyright line for?
And above change updates Copyright from 2012 to 2012-2017, is this correct?

The year line was changes in order to show when the file was changed the last time...

quoted

  *
  *   Redistribution and use in source and binary forms, with or without
  *   modification, are permitted provided that the following conditions

@@ -68,10 +68,6 @@
 #pragma GCC diagnostic error "-Wpedantic"
 #endif

Above invisible lines are  "#include <infiniband/verbs.h>" wrapped with
#pragma for pedantic.

That piece moved to "mlx4.h" [1], which included a few lines later, so can
these line be removed from this line?

quoted

-/* DPDK headers don't like -pedantic. */ -#ifdef PEDANTIC -#pragma
GCC diagnostic ignored "-Wpedantic"
-#endif

Comment says "DPDK headers don't like -pedantic", won't removing
#pragma cause compile error with pedantic option?

It is not necessary anymore, was fixed with the next commit:

commit c0362128c57a0ad22ea311a9657bb15a44b70793
Author: Adrien Mazarguil [off-list ref]
Date:   Mon Jun 29 11:34:52 2015 +0200

    eal: fix pedantic build of mlx4 debug mode

quoted

 #include <rte_ether.h>
 #include <rte_ethdev.h>
 #include <rte_dev.h>

@@ -86,9 +82,6 @@
 #include <rte_log.h>
 #include <rte_alarm.h>
 #include <rte_memory.h>
-#ifdef PEDANTIC
-#pragma GCC diagnostic error "-Wpedantic"
-#endif

 /* Generated configuration header. */  #include "mlx4_autoconf.h"

@@ -96,21 +89,6 @@
 /* PMD header. */
 #include "mlx4.h"

<...>

quoted

diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h index

4c7505e..70c9ecd 100644

--- a/drivers/net/mlx4/mlx4.h
+++ b/drivers/net/mlx4/mlx4.h

<...>

quoted

+
+/* Verbs header. */
+/* ISO C doesn't support unnamed structs/unions, disabling -pedantic.
+*/ #ifdef PEDANTIC #pragma GCC diagnostic ignored "-Wpedantic"
+#endif
+#include <infiniband/verbs.h>
+#ifdef PEDANTIC
+#pragma GCC diagnostic error "-Wpedantic"
+#endif

--> [1]

<...>

quoted

+
+void priv_lock(struct priv *priv);
+void priv_unlock(struct priv *priv);

It can be good to mention in commit log that these functions are now
exported.

quoted

+
 #endif /* RTE_PMD_MLX4_H_ */

I will fix the rest of the issues and will send the v3 patches.

Thank you,
Vasily