[PATCH] make device.c aware of win x64 file descriptors

  • From: Immanuel Weber <immanuel.weber@xxxxxxxxxxxxxxxxx>
  • Date: Thu, 26 Sep 2013 15:55:51 +0200

introduced new fd.h header for platform dependent typedefs for file
descriptors
---
 src/CMakeLists.txt            |  1 +
 src/core/device.c             | 24 ++++++++++++------------
 src/utils/efd.h               |  2 ++
 src/utils/efd_eventfd.h       |  2 --
 src/utils/efd_pipe.h          |  2 --
 src/utils/efd_win.h           |  2 --
 src/utils/{efd_win.h => fd.h} | 17 +++++++++--------
 7 files changed, 24 insertions(+), 26 deletions(-)
 copy src/utils/{efd_win.h => fd.h} (85%)

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index a57dc42..c6089c1 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -108,6 +108,7 @@ set (NN_SOURCES
     utils/err.h
     utils/err.c
     utils/fast.h
+    utils/fd.h
     utils/glock.h
     utils/glock.c
     utils/hash.h
diff --git a/src/core/device.c b/src/core/device.c
index e555e20..abbf0cc 100644
--- a/src/core/device.c
+++ b/src/core/device.c
@@ -24,6 +24,7 @@

 #include "../utils/err.h"
 #include "../utils/fast.h"
+#include "../utils/fd.h"

 #include <string.h>

@@ -37,9 +38,8 @@

 /*  Private functions. */
 static int nn_device_loopback (int s);
-static int nn_device_twoway (int s1, int s1rcv, int s1snd,
-    int s2, int s2rcv, int s2snd);
-static int nn_device_oneway (int s1, int s1rcv, int s2, int s2snd);
+static int nn_device_twoway (int s1, nn_fd s1rcv, nn_fd s1snd, int s2,
nn_fd s2rcv, nn_fd s2snd);
+static int nn_device_oneway (int s1, nn_fd s1rcv, int s2, nn_fd s2snd);
 static int nn_device_mvmsg (int from, int to, int flags);

 int nn_device (int s1, int s2)
@@ -47,10 +47,10 @@ int nn_device (int s1, int s2)
     int rc;
     int op1;
     int op2;
-    int s1rcv;
-    int s1snd;
-    int s2rcv;
-    int s2snd;
+    nn_fd s1rcv;
+    nn_fd s1snd;
+    nn_fd s2rcv;
+    nn_fd s2snd;
     size_t opsz;

     /*  At least one socket must be specified. */
@@ -190,8 +190,8 @@ int nn_device_loopback (int s)

 #if defined NN_HAVE_WINDOWS

-static int nn_device_twoway (int s1, int s1rcv, int s1snd,
-    int s2, int s2rcv, int s2snd)
+static int nn_device_twoway (int s1, nn_fd s1rcv, nn_fd s1snd,
+    int s2, nn_fd s2rcv, nn_fd s2snd)
 {
     int rc;
     fd_set fds;
@@ -256,8 +256,8 @@ static int nn_device_twoway (int s1, int s1rcv, int
s1snd,

 #elif defined NN_HAVE_POLL

-static int nn_device_twoway (int s1, int s1rcv, int s1snd,
-    int s2, int s2rcv, int s2snd)
+static int nn_device_twoway (int s1, nn_fd s1rcv, nn_fd s1snd,
+    int s2, nn_fd s2rcv, nn_fd s2snd)
 {
     int rc;
     struct pollfd pfd [4];
@@ -316,7 +316,7 @@ static int nn_device_twoway (int s1, int s1rcv, int
s1snd,
 #error
 #endif

-static int nn_device_oneway (int s1, int s1rcv, int s2, int s2snd)
+static int nn_device_oneway (int s1, nn_fd s1rcv, int s2, nn_fd s2snd)
 {
     int rc;

diff --git a/src/utils/efd.h b/src/utils/efd.h
index 8865f14..6980187 100644
--- a/src/utils/efd.h
+++ b/src/utils/efd.h
@@ -27,6 +27,8 @@
     is that nn_efd_getfd() returns an actual OS-level file descriptor that
     you can poll on to wait for the event. */

+#include "fd.h"
+
 #if defined NN_HAVE_WINDOWS
 #include "efd_win.h"
 #elif defined NN_HAVE_EVENTFD
diff --git a/src/utils/efd_eventfd.h b/src/utils/efd_eventfd.h
index 210e678..e6d090e 100644
--- a/src/utils/efd_eventfd.h
+++ b/src/utils/efd_eventfd.h
@@ -20,8 +20,6 @@
     IN THE SOFTWARE.
 */

-typedef int nn_fd;
-
 struct nn_efd {
     int efd;
 };
diff --git a/src/utils/efd_pipe.h b/src/utils/efd_pipe.h
index 5017a61..405dc81 100644
--- a/src/utils/efd_pipe.h
+++ b/src/utils/efd_pipe.h
@@ -20,8 +20,6 @@
     IN THE SOFTWARE.
 */

-typedef int nn_fd;
-
 struct nn_efd {
     int r;
     int w;
diff --git a/src/utils/efd_win.h b/src/utils/efd_win.h
index c0343ec..cfd5f47 100644
--- a/src/utils/efd_win.h
+++ b/src/utils/efd_win.h
@@ -22,8 +22,6 @@

 #include "win.h"

-typedef SOCKET nn_fd;
-
 struct nn_efd {
     SOCKET r;
     SOCKET w;
diff --git a/src/utils/efd_win.h b/src/utils/fd.h
similarity index 85%
copy from src/utils/efd_win.h
copy to src/utils/fd.h
index c0343ec..0c048c7 100644
--- a/src/utils/efd_win.h
+++ b/src/utils/fd.h
@@ -1,5 +1,5 @@
 /*
-    Copyright (c) 2012-2013 250bpm s.r.o.  All rights reserved.
+    Copyright (c) 2013 Immanuel Weber, Fraunhofer FHR/AMLS  All rights
reserved.

     Permission is hereby granted, free of charge, to any person obtaining
a copy
     of this software and associated documentation files (the "Software"),
@@ -20,13 +20,14 @@
     IN THE SOFTWARE.
 */

-#include "win.h"
+#ifndef NN_FD_INCLUDED
+#define NN_FD_INCLUDED

+#ifdef NN_HAVE_WINDOWS
+#include "win.h"
 typedef SOCKET nn_fd;
+#else
+typedef int nn_fd;
+#endif

-struct nn_efd {
-    SOCKET r;
-    SOCKET w;
-    fd_set fds;
-};
-
+#endif
\ No newline at end of file
-- 
1.8.3.msysgit.0



2013/9/26 Martin Sustrik <sustrik@xxxxxxxxxx>

> On 26/09/13 11:07, Immanuel Weber wrote:
>
>> Hi Martin,
>>
>> yesterday I allmost finished that modification. This morning a simpler
>> solution came to my mind: just include the efd.h header in device.c and
>> exchange the use of int for the file descriptors there from to nn_fd.
>> The patch for that is below, if you prefer your proposed solution, I can
>> provide a patch for that too.
>>
>
> Yes, please. There's no point in introducing a dependency between efd and
> device. Let them rather both depend on nn_fd.
>
>
>  p.s. there are some compiler warnings about conversions scattered across
>> the project (regardless of the patch), where data loss can happen. Some
>> of them are quite severe, like size_t to uint8_t, which is on my machine
>> a 8 to 1 byte conversion. I will try have a look at some of them.
>>
>
> Right. I would say an assert and an explicit cast would work in such case:
>
> size_t sz;
> ...
> nn_assert (sz < 256);
> uint8_t c = (uint8_t) sz;
>
> Martin
>

--089e0122797a8ba3ac04e749c627
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">Hi Martin,<div><br></div><div>here comes the patch (under =
MIT license). Git somehow decided that the new fd.h is so similar to efd_wi=
n.h that it must be a copy of it. So the action of the patch is a little bi=
t unusual I think, but it does what it should.</div>
<div><br></div><div>Greetings</div><div>Immanuel=A0</div><div><br></div><di=
v><div>From cc6768d9257aa86a06f6c6f9249b72925d2d5d95 Mon Sep 17 00:00:00 20=
01</div><div>From: Immanuel Weber &lt;<a href=3D"mailto:immanuel.weber@fhr.=
fraunhofer.de">immanuel.weber@xxxxxxxxxxxxxxxxx</a>&gt;</div>
<div>Date: Thu, 26 Sep 2013 15:55:51 +0200</div><div>Subject: [PATCH] make =
device.c aware of win x64 file descriptors</div><div><br></div><div>introdu=
ced new fd.h header for platform dependent typedefs for file descriptors</d=
iv>
<div>---</div><div>=A0src/CMakeLists.txt =A0 =A0 =A0 =A0 =A0 =A0| =A01 +</d=
iv><div>=A0src/core/device.c =A0 =A0 =A0 =A0 =A0 =A0 | 24 ++++++++++++-----=
-------</div><div>=A0src/utils/efd.h =A0 =A0 =A0 =A0 =A0 =A0 =A0 | =A02 ++<=
/div><div>=A0src/utils/efd_eventfd.h =A0 =A0 =A0 | =A02 --</div>
<div>=A0src/utils/efd_pipe.h =A0 =A0 =A0 =A0 =A0| =A02 --</div><div>=A0src/=
utils/efd_win.h =A0 =A0 =A0 =A0 =A0 | =A02 --</div><div>=A0src/utils/{efd_w=
in.h =3D&gt; fd.h} | 17 +++++++++--------</div><div>=A07 files changed, 24 =
insertions(+), 26 deletions(-)</div>
<div>=A0copy src/utils/{efd_win.h =3D&gt; fd.h} (85%)</div><div><br></div><=
div>diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt</div><div>index a5=
7dc42..c6089c1 100644</div><div>--- a/src/CMakeLists.txt</div><div>+++ b/sr=
c/CMakeLists.txt</div>
<div>@@ -108,6 +108,7 @@ set (NN_SOURCES</div><div>=A0 =A0 =A0utils/err.h</=
div><div>=A0 =A0 =A0utils/err.c</div><div>=A0 =A0 =A0utils/fast.h</div><div=
>+ =A0 =A0utils/fd.h</div><div>=A0 =A0 =A0utils/glock.h</div><div>=A0 =A0 =
=A0utils/glock.c</div><div>=A0 =A0 =A0utils/hash.h</div>
<div>diff --git a/src/core/device.c b/src/core/device.c</div><div>index e55=
5e20..abbf0cc 100644</div><div>--- a/src/core/device.c</div><div>+++ b/src/=
core/device.c</div><div>@@ -24,6 +24,7 @@</div><div>=A0</div><div>=A0#inclu=
de &quot;../utils/err.h&quot;</div>
<div>=A0#include &quot;../utils/fast.h&quot;</div><div>+#include &quot;../u=
tils/fd.h&quot;</div><div>=A0</div><div>=A0#include &lt;string.h&gt;</div><=
div>=A0</div><div>@@ -37,9 +38,8 @@</div><div>=A0</div><div>=A0/* =A0Privat=
e functions. */</div>
<div>=A0static int nn_device_loopback (int s);</div><div>-static int nn_dev=
ice_twoway (int s1, int s1rcv, int s1snd,</div><div>- =A0 =A0int s2, int s2=
rcv, int s2snd);</div><div>-static int nn_device_oneway (int s1, int s1rcv,=
 int s2, int s2snd);</div>
<div>+static int nn_device_twoway (int s1, nn_fd s1rcv, nn_fd s1snd, int s2=
, nn_fd s2rcv, nn_fd s2snd);</div><div>+static int nn_device_oneway (int s1=
, nn_fd s1rcv, int s2, nn_fd s2snd);</div><div>=A0static int nn_device_mvms=
g (int from, int to, int flags);</div>
<div>=A0</div><div>=A0int nn_device (int s1, int s2)</div><div>@@ -47,10 +4=
7,10 @@ int nn_device (int s1, int s2)</div><div>=A0 =A0 =A0int rc;</div><d=
iv>=A0 =A0 =A0int op1;</div><div>=A0 =A0 =A0int op2;</div><div>- =A0 =A0int=
 s1rcv;</div><div>- =A0 =A0int s1snd;</div>
<div>- =A0 =A0int s2rcv;</div><div>- =A0 =A0int s2snd;</div><div>+ =A0 =A0n=
n_fd s1rcv;</div><div>+ =A0 =A0nn_fd s1snd;</div><div>+ =A0 =A0nn_fd s2rcv;=
</div><div>+ =A0 =A0nn_fd s2snd;</div><div>=A0 =A0 =A0size_t opsz;</div><di=
v>=A0</div><div>=A0 =A0 =A0/* =A0At least one socket must be specified. */<=
/div>
<div>@@ -190,8 +190,8 @@ int nn_device_loopback (int s)</div><div>=A0</div>=
<div>=A0#if defined NN_HAVE_WINDOWS</div><div>=A0</div><div>-static int nn_=
device_twoway (int s1, int s1rcv, int s1snd,</div><div>- =A0 =A0int s2, int=
 s2rcv, int s2snd)</div>
<div>+static int nn_device_twoway (int s1, nn_fd s1rcv, nn_fd s1snd,</div><=
div>+ =A0 =A0int s2, nn_fd s2rcv, nn_fd s2snd)</div><div>=A0{</div><div>=A0=
 =A0 =A0int rc;</div><div>=A0 =A0 =A0fd_set fds;</div><div>@@ -256,8 +256,8=
 @@ static int nn_device_twoway (int s1, int s1rcv, int s1snd,</div>
<div>=A0</div><div>=A0#elif defined NN_HAVE_POLL</div><div>=A0</div><div>-s=
tatic int nn_device_twoway (int s1, int s1rcv, int s1snd,</div><div>- =A0 =
=A0int s2, int s2rcv, int s2snd)</div><div>+static int nn_device_twoway (in=
t s1, nn_fd s1rcv, nn_fd s1snd,</div>
<div>+ =A0 =A0int s2, nn_fd s2rcv, nn_fd s2snd)</div><div>=A0{</div><div>=
=A0 =A0 =A0int rc;</div><div>=A0 =A0 =A0struct pollfd pfd [4];</div><div>@@=
 -316,7 +316,7 @@ static int nn_device_twoway (int s1, int s1rcv, int s1snd=
,</div><div>=A0#error</div>
<div>=A0#endif</div><div>=A0</div><div>-static int nn_device_oneway (int s1=
, int s1rcv, int s2, int s2snd)</div><div>+static int nn_device_oneway (int=
 s1, nn_fd s1rcv, int s2, nn_fd s2snd)</div><div>=A0{</div><div>=A0 =A0 =A0=
int rc;</div>
<div>=A0</div><div>diff --git a/src/utils/efd.h b/src/utils/efd.h</div><div=
>index 8865f14..6980187 100644</div><div>--- a/src/utils/efd.h</div><div>++=
+ b/src/utils/efd.h</div><div>@@ -27,6 +27,8 @@</div><div>=A0 =A0 =A0is tha=
t nn_efd_getfd() returns an actual OS-level file descriptor that</div>
<div>=A0 =A0 =A0you can poll on to wait for the event. */</div><div>=A0</di=
v><div>+#include &quot;fd.h&quot;</div><div>+</div><div>=A0#if defined NN_H=
AVE_WINDOWS</div><div>=A0#include &quot;efd_win.h&quot;</div><div>=A0#elif =
defined NN_HAVE_EVENTFD</div>
<div>diff --git a/src/utils/efd_eventfd.h b/src/utils/efd_eventfd.h</div><d=
iv>index 210e678..e6d090e 100644</div><div>--- a/src/utils/efd_eventfd.h</d=
iv><div>+++ b/src/utils/efd_eventfd.h</div><div>@@ -20,8 +20,6 @@</div>
<div>=A0 =A0 =A0IN THE SOFTWARE.</div><div>=A0*/</div><div>=A0</div><div>-t=
ypedef int nn_fd;</div><div>-</div><div>=A0struct nn_efd {</div><div>=A0 =
=A0 =A0int efd;</div><div>=A0};</div><div>diff --git a/src/utils/efd_pipe.h=
 b/src/utils/efd_pipe.h</div>
<div>index 5017a61..405dc81 100644</div><div>--- a/src/utils/efd_pipe.h</di=
v><div>+++ b/src/utils/efd_pipe.h</div><div>@@ -20,8 +20,6 @@</div><div>=A0=
 =A0 =A0IN THE SOFTWARE.</div><div>=A0*/</div><div>=A0</div><div>-typedef i=
nt nn_fd;</div>
<div>-</div><div>=A0struct nn_efd {</div><div>=A0 =A0 =A0int r;</div><div>=
=A0 =A0 =A0int w;</div><div>diff --git a/src/utils/efd_win.h b/src/utils/ef=
d_win.h</div><div>index c0343ec..cfd5f47 100644</div><div>--- a/src/utils/e=
fd_win.h</div>
<div>+++ b/src/utils/efd_win.h</div><div>@@ -22,8 +22,6 @@</div><div>=A0</d=
iv><div>=A0#include &quot;win.h&quot;</div><div>=A0</div><div>-typedef SOCK=
ET nn_fd;</div><div>-</div><div>=A0struct nn_efd {</div><div>=A0 =A0 =A0SOC=
KET r;</div>
<div>=A0 =A0 =A0SOCKET w;</div><div>diff --git a/src/utils/efd_win.h b/src/=
utils/fd.h</div><div>similarity index 85%</div><div>copy from src/utils/efd=
_win.h</div><div>copy to src/utils/fd.h</div><div>index c0343ec..0c048c7 10=
0644</div>
<div>--- a/src/utils/efd_win.h</div><div>+++ b/src/utils/fd.h</div><div>@@ =
-1,5 +1,5 @@</div><div>=A0/*</div><div>- =A0 =A0Copyright (c) 2012-2013 250=
bpm s.r.o. =A0All rights reserved.</div><div>+ =A0 =A0Copyright (c) 2013 Im=
manuel Weber, Fraunhofer FHR/AMLS =A0All rights reserved.</div>
<div>=A0</div><div>=A0 =A0 =A0Permission is hereby granted, free of charge,=
 to any person obtaining a copy</div><div>=A0 =A0 =A0of this software and a=
ssociated documentation files (the &quot;Software&quot;),</div><div>@@ -20,=
13 +20,14 @@</div>
<div>=A0 =A0 =A0IN THE SOFTWARE.</div><div>=A0*/</div><div>=A0</div><div>-#=
include &quot;win.h&quot;</div><div>+#ifndef NN_FD_INCLUDED</div><div>+#def=
ine NN_FD_INCLUDED</div><div>=A0</div><div>+#ifdef NN_HAVE_WINDOWS</div><di=
v>+#include &quot;win.h&quot;</div>
<div>=A0typedef SOCKET nn_fd;</div><div>+#else</div><div>+typedef int nn_fd=
;</div><div>+#endif</div><div>=A0</div><div>-struct nn_efd {</div><div>- =
=A0 =A0SOCKET r;</div><div>- =A0 =A0SOCKET w;</div><div>- =A0 =A0fd_set fds=
;</div><div>-};</div>
<div>-</div><div>+#endif</div><div>\ No newline at end of file</div><div>--=
=A0</div><div>1.8.3.msysgit.0</div><div><br></div></div></div><div class=3D=
"gmail_extra"><br><br><div class=3D"gmail_quote">2013/9/26 Martin Sustrik <=
span dir=3D"ltr">&lt;<a href=3D"mailto:sustrik@xxxxxxxxxx"; target=3D"_blank=
">sustrik@xxxxxxxxxx</a>&gt;</span><br>
<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex"><div class=3D"im">On 26/09/13 11:07, Immanue=
l Weber wrote:<br>
<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex">
Hi Martin,<br>
<br>
yesterday I allmost finished that modification. This morning a simpler<br>
solution came to my mind: just include the efd.h header in device.c and<br>
exchange the use of int for the file descriptors there from to nn_fd.<br>
The patch for that is below, if you prefer your proposed solution, I can<br=
>
provide a patch for that too.<br>
</blockquote>
<br></div>
Yes, please. There&#39;s no point in introducing a dependency between efd a=
nd device. Let them rather both depend on nn_fd.<div class=3D"im"><br>
<br>
<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex">
p.s. there are some compiler warnings about conversions scattered across<br=
>
the project (regardless of the patch), where data loss can happen. Some<br>
of them are quite severe, like size_t to uint8_t, which is on my machine<br=
>
a 8 to 1 byte conversion. I will try have a look at some of them.<br>
</blockquote>
<br></div>
Right. I would say an assert and an explicit cast would work in such case:<=
br>
<br>
size_t sz;<br>
...<br>
nn_assert (sz &lt; 256);<br>
uint8_t c =3D (uint8_t) sz;<br>
<br>
Martin<br>
</blockquote></div><br></div>

--089e0122797a8ba3ac04e749c627--

Other related posts: