[dpdk-dev] net/virtio-user: fix O_CLOEXEC undeclared error

Message ID 1466948986-12255-1-git-send-email-jianfeng.tan@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Yuanhan Liu
Headers

Commit Message

Jianfeng Tan June 26, 2016, 1:49 p.m. UTC
  On some older systems, such as SUSE 11, the compiling error shows
as:
   .../dpdk/drivers/net/virtio/virtio_user/virtio_user_dev.c:67:22:
         error: ‘O_CLOEXEC’ undeclared (first use in this function)

The fix is to declare _GNU_SOURCE macro before include fcntl.h.

Fixes: 37a7eb2ae816 ("net/virtio-user: add device emulation layer")

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/net/virtio/virtio_user/virtio_user_dev.c | 1 +
 1 file changed, 1 insertion(+)
  

Comments

Yuanhan Liu June 28, 2016, 9:02 a.m. UTC | #1
On Sun, Jun 26, 2016 at 01:49:46PM +0000, Jianfeng Tan wrote:
> On some older systems, such as SUSE 11, the compiling error shows
> as:
>    .../dpdk/drivers/net/virtio/virtio_user/virtio_user_dev.c:67:22:
>          error: ‘O_CLOEXEC’ undeclared (first use in this function)
> 
> The fix is to declare _GNU_SOURCE macro before include fcntl.h.

I think you need give a reason how that error comes and why this
would fix the error.

And if you look at the git history, you might want to define this
macro in Makefile. See commit e49680a87e06 ("mk: compilation fixes").

Last, don't forgot to run ./scripts/check-git-log.sh against your patch.

	--yliu
  
Ferruh Yigit June 28, 2016, 9:41 a.m. UTC | #2
On 6/26/2016 2:49 PM, Jianfeng Tan wrote:
> On some older systems, such as SUSE 11, the compiling error shows
> as:
>    .../dpdk/drivers/net/virtio/virtio_user/virtio_user_dev.c:67:22:
>          error: ‘O_CLOEXEC’ undeclared (first use in this function)
> 
> The fix is to declare _GNU_SOURCE macro before include fcntl.h.
> 
> Fixes: 37a7eb2ae816 ("net/virtio-user: add device emulation layer")
> 
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
>  drivers/net/virtio/virtio_user/virtio_user_dev.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index 3d12a32..180f824 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -31,6 +31,7 @@
>   *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>   */
>  
> +#define _GNU_SOURCE
This flag enables glibc extensions, and kind of problem on code
portability. How big problem is it?
Is there any platform that DPDK runs on which uses a libc that isn't
compatible with what this flag provides?

And it seems this flag already used in DPDK, including libraries. If we
are agree on using glibc extensions, does it make sense to move this
flag into a single common location (like mk/exec-env/) ?

>  #include <stdint.h>
>  #include <stdio.h>
>  #include <fcntl.h>
>
  
Yuanhan Liu June 29, 2016, 1:56 a.m. UTC | #3
On Tue, Jun 28, 2016 at 10:41:24AM +0100, Ferruh Yigit wrote:
> On 6/26/2016 2:49 PM, Jianfeng Tan wrote:
> > On some older systems, such as SUSE 11, the compiling error shows
> > as:
> >    .../dpdk/drivers/net/virtio/virtio_user/virtio_user_dev.c:67:22:
> >          error: ‘O_CLOEXEC’ undeclared (first use in this function)
> > 
> > The fix is to declare _GNU_SOURCE macro before include fcntl.h.
> > 
> > Fixes: 37a7eb2ae816 ("net/virtio-user: add device emulation layer")
> > 
> > Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> > ---
> >  drivers/net/virtio/virtio_user/virtio_user_dev.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > index 3d12a32..180f824 100644
> > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > @@ -31,6 +31,7 @@
> >   *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> >   */
> >  
> > +#define _GNU_SOURCE
> This flag enables glibc extensions, and kind of problem on code
> portability. How big problem is it?

Honestly, I don't know. But I was also thinking that define _GNU_SOURCE
here is an overkill to me. For this issue, we may simply fix it by
reference the O_CLOEXEC flag only when it's defined.

> Is there any platform that DPDK runs on which uses a libc that isn't
> compatible with what this flag provides?
> 
> And it seems this flag already used in DPDK, including libraries.

Yes, "git grep _GNU_SOURCE" shows a lot of such usage.

	--yliu

> If we
> are agree on using glibc extensions, does it make sense to move this
> flag into a single common location (like mk/exec-env/) ?
> 
> >  #include <stdint.h>
> >  #include <stdio.h>
> >  #include <fcntl.h>
> >
  

Patch

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 3d12a32..180f824 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -31,6 +31,7 @@ 
  *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#define _GNU_SOURCE
 #include <stdint.h>
 #include <stdio.h>
 #include <fcntl.h>