* [Qemu-devel] [PATCH] slirp: reorder include to fix FreeBSD build failure
@ 2013-07-12 20:29 Ed Maste
2013-07-13 9:12 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
0 siblings, 1 reply; 7+ messages in thread
From: Ed Maste @ 2013-07-12 20:29 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, Ed Maste
Signed-off-by: Ed Maste <emaste@freebsd.org>
---
The issue comes from slirp/mbuf.h #defining m_flags, which conflicts with
a header included via <semaphore.h> on FreeBSD.
I'm not sure exactly when this broke; I assume that it used to work because
something previously caused the conflicting header to be included earlier,
and that has now been removed.
slirp/slirp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 80b28ea..231afc8 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -24,8 +24,8 @@
#include "qemu-common.h"
#include "qemu/timer.h"
#include "sysemu/char.h"
-#include "slirp.h"
#include "hw/hw.h"
+#include "slirp.h"
/* host loopback address */
struct in_addr loopback_addr;
--
1.7.11.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] slirp: reorder include to fix FreeBSD build failure
2013-07-12 20:29 [Qemu-devel] [PATCH] slirp: reorder include to fix FreeBSD build failure Ed Maste
@ 2013-07-13 9:12 ` Michael Tokarev
2013-07-13 22:35 ` Ed Maste
0 siblings, 1 reply; 7+ messages in thread
From: Michael Tokarev @ 2013-07-13 9:12 UTC (permalink / raw)
To: Ed Maste; +Cc: qemu-trivial, Jan Kiszka, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1732 bytes --]
13.07.2013 00:29, Ed Maste wrote:
> Signed-off-by: Ed Maste <emaste@freebsd.org>
> ---
> The issue comes from slirp/mbuf.h #defining m_flags, which conflicts with
> a header included via <semaphore.h> on FreeBSD.
Umgh. How.. disgusting... :(
Here's the code in question.
/* header at beginning of each mbuf: */
struct m_hdr {
struct mbuf *mh_next; /* Linked list of mbufs */
struct mbuf *mh_prev;
struct mbuf *mh_nextpkt; /* Next packet in queue/record */
struct mbuf *mh_prevpkt; /* Flags aren't used in the output queue */
int mh_flags; /* Misc flags */
...
};
struct mbuf {
struct m_hdr m_hdr;
Slirp *slirp;
bool arp_requested;
...
};
#define m_next m_hdr.mh_next
#define m_prev m_hdr.mh_prev
#define m_nextpkt m_hdr.mh_nextpkt
#define m_prevpkt m_hdr.mh_prevpkt
#define m_flags m_hdr.mh_flags
...
It looks to me that we should just get rid of all this.
struct m_hdr isn't used anywhere else so all its
members can be included directly into struct mbuf,
removing a good portion of those disgusting #defines.
Remaining:
struct mbuf {
union M_dat {
char m_dat_[1]; /* ANSI don't like 0 sized arrays */
char *m_ext_;
} M_dat;
};
#define m_dat M_dat.m_dat_
#define m_ext M_dat.m_ext_
This can be done by using an unnamed union, ie, by omitting
M_dat.
And since the code almost everywhere uses those m_* defines, it
is trivial to fix this for real.
Something like the attached.
Maybe we should get rid of the rest of indirections here too,
ifq_prev ifs_prev etc, but i'm not sure about these.
Thanks,
/mjt
[-- Attachment #2: slirp-remove-mbuf-m_hdr-m_dat-indirection.patch --]
[-- Type: text/x-patch, Size: 5606 bytes --]
>From ca96db1db67a6d244ce983450929a0c32c26a9cd Mon Sep 17 00:00:00 2001
From: Michael Tokarev <mjt@tls.msk.ru>
Date: Sat, 13 Jul 2013 13:10:05 +0400
Subject: [PATCH] slirp: remove mbuf(m_hdr,m_dat) indirection
Signed-off-By: Michael Tokarev <mjt@tls.msk.ru>
---
slirp/mbuf.h | 51 ++++++++++++++++++---------------------------------
slirp/tcp_subr.c | 12 ++++++------
2 files changed, 24 insertions(+), 39 deletions(-)
diff --git a/slirp/mbuf.h b/slirp/mbuf.h
index 3f3ab09..b144f1c 100644
--- a/slirp/mbuf.h
+++ b/slirp/mbuf.h
@@ -49,22 +49,6 @@
* free the m_ext. This is inefficient memory-wise, but who cares.
*/
-/* XXX should union some of these! */
-/* header at beginning of each mbuf: */
-struct m_hdr {
- struct mbuf *mh_next; /* Linked list of mbufs */
- struct mbuf *mh_prev;
- struct mbuf *mh_nextpkt; /* Next packet in queue/record */
- struct mbuf *mh_prevpkt; /* Flags aren't used in the output queue */
- int mh_flags; /* Misc flags */
-
- int mh_size; /* Size of data */
- struct socket *mh_so;
-
- caddr_t mh_data; /* Location of data */
- int mh_len; /* Amount of data in this mbuf */
-};
-
/*
* How much room is in the mbuf, from m_data to the end of the mbuf
*/
@@ -80,29 +64,30 @@ struct m_hdr {
#define M_TRAILINGSPACE M_FREEROOM
struct mbuf {
- struct m_hdr m_hdr;
+ /* XXX should union some of these! */
+ /* header at beginning of each mbuf: */
+ struct mbuf *m_next; /* Linked list of mbufs */
+ struct mbuf *m_prev;
+ struct mbuf *m_nextpkt; /* Next packet in queue/record */
+ struct mbuf *m_prevpkt; /* Flags aren't used in the output queue */
+ int m_flags; /* Misc flags */
+
+ int m_size; /* Size of data */
+ struct socket *m_so;
+
+ caddr_t m_data; /* Location of data */
+ int m_len; /* Amount of data in this mbuf */
+
Slirp *slirp;
bool arp_requested;
uint64_t expiration_date;
/* start of dynamic buffer area, must be last element */
- union M_dat {
- char m_dat_[1]; /* ANSI don't like 0 sized arrays */
- char *m_ext_;
- } M_dat;
+ union {
+ char m_dat[1]; /* ANSI don't like 0 sized arrays */
+ char *m_ext;
+ };
};
-#define m_next m_hdr.mh_next
-#define m_prev m_hdr.mh_prev
-#define m_nextpkt m_hdr.mh_nextpkt
-#define m_prevpkt m_hdr.mh_prevpkt
-#define m_flags m_hdr.mh_flags
-#define m_len m_hdr.mh_len
-#define m_data m_hdr.mh_data
-#define m_size m_hdr.mh_size
-#define m_dat M_dat.m_dat_
-#define m_ext M_dat.m_ext_
-#define m_so m_hdr.mh_so
-
#define ifq_prev m_prev
#define ifq_next m_next
#define ifs_prev m_prevpkt
diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
index e98ce1a..043f28f 100644
--- a/slirp/tcp_subr.c
+++ b/slirp/tcp_subr.c
@@ -647,7 +647,7 @@ tcp_emu(struct socket *so, struct mbuf *m)
n4 = (laddr & 0xff);
m->m_len = bptr - m->m_data; /* Adjust length */
- m->m_len += snprintf(bptr, m->m_hdr.mh_size - m->m_len,
+ m->m_len += snprintf(bptr, m->m_size - m->m_len,
"ORT %d,%d,%d,%d,%d,%d\r\n%s",
n1, n2, n3, n4, n5, n6, x==7?buff:"");
return 1;
@@ -680,7 +680,7 @@ tcp_emu(struct socket *so, struct mbuf *m)
n4 = (laddr & 0xff);
m->m_len = bptr - m->m_data; /* Adjust length */
- m->m_len += snprintf(bptr, m->m_hdr.mh_size - m->m_len,
+ m->m_len += snprintf(bptr, m->m_size - m->m_len,
"27 Entering Passive Mode (%d,%d,%d,%d,%d,%d)\r\n%s",
n1, n2, n3, n4, n5, n6, x==7?buff:"");
@@ -706,7 +706,7 @@ tcp_emu(struct socket *so, struct mbuf *m)
if (m->m_data[m->m_len-1] == '\0' && lport != 0 &&
(so = tcp_listen(slirp, INADDR_ANY, 0, so->so_laddr.s_addr,
htons(lport), SS_FACCEPTONCE)) != NULL)
- m->m_len = snprintf(m->m_data, m->m_hdr.mh_size, "%d",
+ m->m_len = snprintf(m->m_data, m->m_size, "%d",
ntohs(so->so_fport)) + 1;
return 1;
@@ -726,7 +726,7 @@ tcp_emu(struct socket *so, struct mbuf *m)
return 1;
}
m->m_len = bptr - m->m_data; /* Adjust length */
- m->m_len += snprintf(bptr, m->m_hdr.mh_size,
+ m->m_len += snprintf(bptr, m->m_size,
"DCC CHAT chat %lu %u%c\n",
(unsigned long)ntohl(so->so_faddr.s_addr),
ntohs(so->so_fport), 1);
@@ -737,7 +737,7 @@ tcp_emu(struct socket *so, struct mbuf *m)
return 1;
}
m->m_len = bptr - m->m_data; /* Adjust length */
- m->m_len += snprintf(bptr, m->m_hdr.mh_size,
+ m->m_len += snprintf(bptr, m->m_size,
"DCC SEND %s %lu %u %u%c\n", buff,
(unsigned long)ntohl(so->so_faddr.s_addr),
ntohs(so->so_fport), n1, 1);
@@ -748,7 +748,7 @@ tcp_emu(struct socket *so, struct mbuf *m)
return 1;
}
m->m_len = bptr - m->m_data; /* Adjust length */
- m->m_len += snprintf(bptr, m->m_hdr.mh_size,
+ m->m_len += snprintf(bptr, m->m_size,
"DCC MOVE %s %lu %u %u%c\n", buff,
(unsigned long)ntohl(so->so_faddr.s_addr),
ntohs(so->so_fport), n1, 1);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] slirp: reorder include to fix FreeBSD build failure
2013-07-13 9:12 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
@ 2013-07-13 22:35 ` Ed Maste
2013-07-14 10:14 ` Peter Maydell
2013-07-17 8:32 ` Jan Kiszka
0 siblings, 2 replies; 7+ messages in thread
From: Ed Maste @ 2013-07-13 22:35 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-trivial, Jan Kiszka, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 708 bytes --]
On 13 July 2013 05:12, Michael Tokarev <mjt@tls.msk.ru> wrote:
> Remaining:
>
> struct mbuf {
> union M_dat {
> char m_dat_[1]; /* ANSI don't like 0 sized arrays */
> char *m_ext_;
> } M_dat;
> };
>
> #define m_dat M_dat.m_dat_
> #define m_ext M_dat.m_ext_
>
> This can be done by using an unnamed union, ie, by omitting
>
Yeah, struct mbuf and those #defines date back to the beginning of BSD
networking.
I think we're probably unconcerned with a slirp upstream at this point, so
such a change seems reasonable. I'm not sure that anonymous union support
is universal across all compilers used to build QEMU though - do you know?
[-- Attachment #2: Type: text/html, Size: 1099 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] slirp: reorder include to fix FreeBSD build failure
2013-07-13 22:35 ` Ed Maste
@ 2013-07-14 10:14 ` Peter Maydell
2013-07-17 8:32 ` Jan Kiszka
1 sibling, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2013-07-14 10:14 UTC (permalink / raw)
To: Ed Maste; +Cc: qemu-trivial, Jan Kiszka, Michael Tokarev, qemu-devel
On 13 July 2013 23:35, Ed Maste <emaste@freebsd.org> wrote:
> I'm not sure that anonymous union support is universal across
> all compilers used to build QEMU though - do you know?
A quick grep shows we already use anonymous unions in
a few places, so it must be OK.
-- PMM
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] slirp: reorder include to fix FreeBSD build failure
2013-07-13 22:35 ` Ed Maste
2013-07-14 10:14 ` Peter Maydell
@ 2013-07-17 8:32 ` Jan Kiszka
2013-07-17 8:54 ` Michael Tokarev
1 sibling, 1 reply; 7+ messages in thread
From: Jan Kiszka @ 2013-07-17 8:32 UTC (permalink / raw)
To: Ed Maste; +Cc: qemu-trivial, Michael Tokarev, qemu-devel
On 2013-07-14 00:35, Ed Maste wrote:
> On 13 July 2013 05:12, Michael Tokarev <mjt@tls.msk.ru<mailto:mjt@tls.msk.ru>> wrote:
> Remaining:
>
> struct mbuf {
> union M_dat {
> char m_dat_[1]; /* ANSI don't like 0 sized arrays */
> char *m_ext_;
> } M_dat;
> };
>
> #define m_dat M_dat.m_dat_
> #define m_ext M_dat.m_ext_
>
> This can be done by using an unnamed union, ie, by omitting
>
> Yeah, struct mbuf and those #defines date back to the beginning of BSD networking.
>
> I think we're probably unconcerned with a slirp upstream at this point, so such a change seems reasonable.
Indeed, they are always welcome. There is no point in tracking upstream
anymore.
> I'm not sure that anonymous union support is universal across all compilers used to build QEMU though - do you know?
No problem, as Peter already said. Please provide an according patch.
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] slirp: reorder include to fix FreeBSD build failure
2013-07-17 8:32 ` Jan Kiszka
@ 2013-07-17 8:54 ` Michael Tokarev
2013-07-17 14:44 ` Ed Maste
0 siblings, 1 reply; 7+ messages in thread
From: Michael Tokarev @ 2013-07-17 8:54 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-trivial, Ed Maste, qemu-devel
17.07.2013 12:32, Jan Kiszka wrote:
> No problem, as Peter already said. Please provide an according patch.
http://thread.gmane.org/gmane.comp.emulators.qemu/221949/focus=221975
(an attachtment there)
http://git.corpit.ru/?p=qemu.git;a=commitdiff;h=2915c3b20260b1a653fced3b584d0c2b012880dc
Thanks,
/mjt
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] slirp: reorder include to fix FreeBSD build failure
2013-07-17 8:54 ` Michael Tokarev
@ 2013-07-17 14:44 ` Ed Maste
0 siblings, 0 replies; 7+ messages in thread
From: Ed Maste @ 2013-07-17 14:44 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-trivial, Jan Kiszka, qemu-devel
On 17 July 2013 04:54, Michael Tokarev <mjt@tls.msk.ru> wrote:
> 17.07.2013 12:32, Jan Kiszka wrote:
>> No problem, as Peter already said. Please provide an according patch.
>
> http://thread.gmane.org/gmane.comp.emulators.qemu/221949/focus=221975
> (an attachtment there)
> http://git.corpit.ru/?p=qemu.git;a=commitdiff;h=2915c3b20260b1a653fced3b584d0c2b012880dc
I'm happy with Michael's patch.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-07-17 14:44 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-12 20:29 [Qemu-devel] [PATCH] slirp: reorder include to fix FreeBSD build failure Ed Maste
2013-07-13 9:12 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
2013-07-13 22:35 ` Ed Maste
2013-07-14 10:14 ` Peter Maydell
2013-07-17 8:32 ` Jan Kiszka
2013-07-17 8:54 ` Michael Tokarev
2013-07-17 14:44 ` Ed Maste
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.