qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Stefan Weil <sw@weilnetz.de>, Jan Kiszka <jan.kiszka@siemens.com>
Cc: "Jason Wang (jasowang@redhat.com)" <jasowang@redhat.com>,
	QEMU Developer <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH] slirp: Fix type casts and format strings in debug code
Date: Tue, 27 Oct 2015 19:39:54 +0100	[thread overview]
Message-ID: <562FC4FA.50600@redhat.com> (raw)
In-Reply-To: <1440832355-4933-1-git-send-email-sw@weilnetz.de>

Wasn't someone else going to help Jan as SLIRP comaintainer?

Jason, perhaps you can pick up this patch in the meanwhile?

Paolo

On 29/08/2015 09:12, Stefan Weil wrote:
> Casting pointers to long won't work on 64 bit Windows.
> It is not needed with the right format strings.
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  slirp/bootp.c      | 12 +++++++++---
>  slirp/if.c         |  4 ++--
>  slirp/ip_icmp.c    |  4 ++--
>  slirp/ip_input.c   | 10 +++++-----
>  slirp/ip_output.c  |  4 ++--
>  slirp/mbuf.c       |  6 +++---
>  slirp/misc.c       |  6 +++---
>  slirp/sbuf.c       |  4 ++--
>  slirp/socket.c     | 18 +++++++++---------
>  slirp/tcp_input.c  | 14 +++++++-------
>  slirp/tcp_output.c |  2 +-
>  slirp/tcp_subr.c   | 16 ++++++++--------
>  slirp/udp.c        |  6 +++---
>  13 files changed, 56 insertions(+), 50 deletions(-)
> 
> diff --git a/slirp/bootp.c b/slirp/bootp.c
> index b7db9fa..1baaab1 100644
> --- a/slirp/bootp.c
> +++ b/slirp/bootp.c
> @@ -23,6 +23,12 @@
>   */
>  #include <slirp.h>
>  
> +#if defined(_WIN32)
> +/* Windows ntohl() returns an u_long value.
> + * Add a type cast to match the format strings. */
> +# define ntohl(n) ((uint32_t)ntohl(n))
> +#endif
> +
>  /* XXX: only DHCP is supported */
>  
>  #define LEASE_TIME (24 * 3600)
> @@ -155,7 +161,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
>      dhcp_decode(bp, &dhcp_msg_type, &preq_addr);
>      DPRINTF("bootp packet op=%d msgtype=%d", bp->bp_op, dhcp_msg_type);
>      if (preq_addr.s_addr != htonl(0L))
> -        DPRINTF(" req_addr=%08x\n", ntohl(preq_addr.s_addr));
> +        DPRINTF(" req_addr=%08" PRIx32 "\n", ntohl(preq_addr.s_addr));
>      else
>          DPRINTF("\n");
>  
> @@ -234,7 +240,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
>      q += 4;
>  
>      if (bc) {
> -        DPRINTF("%s addr=%08x\n",
> +        DPRINTF("%s addr=%08" PRIx32 "\n",
>                  (dhcp_msg_type == DHCPDISCOVER) ? "offered" : "ack'ed",
>                  ntohl(daddr.sin_addr.s_addr));
>  
> @@ -302,7 +308,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
>      } else {
>          static const char nak_msg[] = "requested address not available";
>  
> -        DPRINTF("nak'ed addr=%08x\n", ntohl(preq_addr.s_addr));
> +        DPRINTF("nak'ed addr=%08" PRIx32 "\n", ntohl(preq_addr.s_addr));
>  
>          *q++ = RFC2132_MSG_TYPE;
>          *q++ = 1;
> diff --git a/slirp/if.c b/slirp/if.c
> index fb7acf8..8325a2a 100644
> --- a/slirp/if.c
> +++ b/slirp/if.c
> @@ -53,8 +53,8 @@ if_output(struct socket *so, struct mbuf *ifm)
>  	int on_fastq = 1;
>  
>  	DEBUG_CALL("if_output");
> -	DEBUG_ARG("so = %lx", (long)so);
> -	DEBUG_ARG("ifm = %lx", (long)ifm);
> +	DEBUG_ARG("so = %p", so);
> +	DEBUG_ARG("ifm = %p", ifm);
>  
>  	/*
>  	 * First remove the mbuf from m_usedlist,
> diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c
> index 9f1cb08..23b9f0f 100644
> --- a/slirp/ip_icmp.c
> +++ b/slirp/ip_icmp.c
> @@ -125,7 +125,7 @@ icmp_input(struct mbuf *m, int hlen)
>    Slirp *slirp = m->slirp;
>  
>    DEBUG_CALL("icmp_input");
> -  DEBUG_ARG("m = %lx", (long )m);
> +  DEBUG_ARG("m = %p", m);
>    DEBUG_ARG("m_len = %d", m->m_len);
>  
>    /*
> @@ -252,7 +252,7 @@ icmp_error(struct mbuf *msrc, u_char type, u_char code, int minsize,
>    register struct mbuf *m;
>  
>    DEBUG_CALL("icmp_error");
> -  DEBUG_ARG("msrc = %lx", (long )msrc);
> +  DEBUG_ARG("msrc = %p", msrc);
>    DEBUG_ARG("msrc_len = %d", msrc->m_len);
>  
>    if(type!=ICMP_UNREACH && type!=ICMP_TIMXCEED) goto end_error;
> diff --git a/slirp/ip_input.c b/slirp/ip_input.c
> index 880bdfd..7d436e6 100644
> --- a/slirp/ip_input.c
> +++ b/slirp/ip_input.c
> @@ -80,7 +80,7 @@ ip_input(struct mbuf *m)
>  	int hlen;
>  
>  	DEBUG_CALL("ip_input");
> -	DEBUG_ARG("m = %lx", (long)m);
> +	DEBUG_ARG("m = %p", m);
>  	DEBUG_ARG("m_len = %d", m->m_len);
>  
>  	if (m->m_len < sizeof (struct ip)) {
> @@ -232,9 +232,9 @@ ip_reass(Slirp *slirp, struct ip *ip, struct ipq *fp)
>  	int i, next;
>  
>  	DEBUG_CALL("ip_reass");
> -	DEBUG_ARG("ip = %lx", (long)ip);
> -	DEBUG_ARG("fp = %lx", (long)fp);
> -	DEBUG_ARG("m = %lx", (long)m);
> +	DEBUG_ARG("ip = %p", ip);
> +	DEBUG_ARG("fp = %p", fp);
> +	DEBUG_ARG("m = %p", m);
>  
>  	/*
>  	 * Presence of header sizes in mbufs
> @@ -400,7 +400,7 @@ static void
>  ip_enq(register struct ipasfrag *p, register struct ipasfrag *prev)
>  {
>  	DEBUG_CALL("ip_enq");
> -	DEBUG_ARG("prev = %lx", (long)prev);
> +	DEBUG_ARG("prev = %p", prev);
>  	p->ipf_prev =  prev;
>  	p->ipf_next = prev->ipf_next;
>  	((struct ipasfrag *)(prev->ipf_next))->ipf_prev = p;
> diff --git a/slirp/ip_output.c b/slirp/ip_output.c
> index c82830f..1254d0d 100644
> --- a/slirp/ip_output.c
> +++ b/slirp/ip_output.c
> @@ -60,8 +60,8 @@ ip_output(struct socket *so, struct mbuf *m0)
>  	int len, off, error = 0;
>  
>  	DEBUG_CALL("ip_output");
> -	DEBUG_ARG("so = %lx", (long)so);
> -	DEBUG_ARG("m0 = %lx", (long)m0);
> +	DEBUG_ARG("so = %p", so);
> +	DEBUG_ARG("m0 = %p", m0);
>  
>  	ip = mtod(m, struct ip *);
>  	/*
> diff --git a/slirp/mbuf.c b/slirp/mbuf.c
> index 4fefb04..795fc29 100644
> --- a/slirp/mbuf.c
> +++ b/slirp/mbuf.c
> @@ -94,7 +94,7 @@ m_get(Slirp *slirp)
>          m->arp_requested = false;
>          m->expiration_date = (uint64_t)-1;
>  end_error:
> -	DEBUG_ARG("m = %lx", (long )m);
> +	DEBUG_ARG("m = %p", m);
>  	return m;
>  }
>  
> @@ -103,7 +103,7 @@ m_free(struct mbuf *m)
>  {
>  
>    DEBUG_CALL("m_free");
> -  DEBUG_ARG("m = %lx", (long )m);
> +  DEBUG_ARG("m = %p", m);
>  
>    if(m) {
>  	/* Remove from m_usedlist */
> @@ -221,7 +221,7 @@ dtom(Slirp *slirp, void *dat)
>  	struct mbuf *m;
>  
>  	DEBUG_CALL("dtom");
> -	DEBUG_ARG("dat = %lx", (long )dat);
> +	DEBUG_ARG("dat = %p", dat);
>  
>  	/* bug corrected for M_EXT buffers */
>  	for (m = slirp->m_usedlist.m_next; m != &slirp->m_usedlist;
> diff --git a/slirp/misc.c b/slirp/misc.c
> index 578e8b2..5497161 100644
> --- a/slirp/misc.c
> +++ b/slirp/misc.c
> @@ -123,9 +123,9 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
>  	pid_t pid;
>  
>  	DEBUG_CALL("fork_exec");
> -	DEBUG_ARG("so = %lx", (long)so);
> -	DEBUG_ARG("ex = %lx", (long)ex);
> -	DEBUG_ARG("do_pty = %lx", (long)do_pty);
> +	DEBUG_ARG("so = %p", so);
> +	DEBUG_ARG("ex = %p", ex);
> +	DEBUG_ARG("do_pty = %x", do_pty);
>  
>  	if (do_pty == 2) {
>                  return 0;
> diff --git a/slirp/sbuf.c b/slirp/sbuf.c
> index 08ec2b4..b8c3db7 100644
> --- a/slirp/sbuf.c
> +++ b/slirp/sbuf.c
> @@ -72,8 +72,8 @@ sbappend(struct socket *so, struct mbuf *m)
>  	int ret = 0;
>  
>  	DEBUG_CALL("sbappend");
> -	DEBUG_ARG("so = %lx", (long)so);
> -	DEBUG_ARG("m = %lx", (long)m);
> +	DEBUG_ARG("so = %p", so);
> +	DEBUG_ARG("m = %p", m);
>  	DEBUG_ARG("m->m_len = %d", m->m_len);
>  
>  	/* Shouldn't happen, but...  e.g. foreign host closes connection */
> diff --git a/slirp/socket.c b/slirp/socket.c
> index 37ac5cf..1673e3a 100644
> --- a/slirp/socket.c
> +++ b/slirp/socket.c
> @@ -91,7 +91,7 @@ size_t sopreprbuf(struct socket *so, struct iovec *iov, int *np)
>  	int mss = so->so_tcpcb->t_maxseg;
>  
>  	DEBUG_CALL("sopreprbuf");
> -	DEBUG_ARG("so = %lx", (long )so);
> +	DEBUG_ARG("so = %p", so);
>  
>  	if (len <= 0)
>  		return 0;
> @@ -155,7 +155,7 @@ soread(struct socket *so)
>  	struct iovec iov[2];
>  
>  	DEBUG_CALL("soread");
> -	DEBUG_ARG("so = %lx", (long )so);
> +	DEBUG_ARG("so = %p", so);
>  
>  	/*
>  	 * No need to check if there's enough room to read.
> @@ -215,7 +215,7 @@ int soreadbuf(struct socket *so, const char *buf, int size)
>  	struct iovec iov[2];
>  
>  	DEBUG_CALL("soreadbuf");
> -	DEBUG_ARG("so = %lx", (long )so);
> +	DEBUG_ARG("so = %p", so);
>  
>  	/*
>  	 * No need to check if there's enough room to read.
> @@ -263,7 +263,7 @@ sorecvoob(struct socket *so)
>  	struct tcpcb *tp = sototcpcb(so);
>  
>  	DEBUG_CALL("sorecvoob");
> -	DEBUG_ARG("so = %lx", (long)so);
> +	DEBUG_ARG("so = %p", so);
>  
>  	/*
>  	 * We take a guess at how much urgent data has arrived.
> @@ -293,7 +293,7 @@ sosendoob(struct socket *so)
>  	int n, len;
>  
>  	DEBUG_CALL("sosendoob");
> -	DEBUG_ARG("so = %lx", (long)so);
> +	DEBUG_ARG("so = %p", so);
>  	DEBUG_ARG("sb->sb_cc = %d", sb->sb_cc);
>  
>  	if (so->so_urgc > 2048)
> @@ -351,7 +351,7 @@ sowrite(struct socket *so)
>  	struct iovec iov[2];
>  
>  	DEBUG_CALL("sowrite");
> -	DEBUG_ARG("so = %lx", (long)so);
> +	DEBUG_ARG("so = %p", so);
>  
>  	if (so->so_urgc) {
>  		sosendoob(so);
> @@ -441,7 +441,7 @@ sorecvfrom(struct socket *so)
>  	socklen_t addrlen = sizeof(struct sockaddr_in);
>  
>  	DEBUG_CALL("sorecvfrom");
> -	DEBUG_ARG("so = %lx", (long)so);
> +	DEBUG_ARG("so = %p", so);
>  
>  	if (so->so_type == IPPROTO_ICMP) {   /* This is a "ping" reply */
>  	  char buff[256];
> @@ -543,8 +543,8 @@ sosendto(struct socket *so, struct mbuf *m)
>  	struct sockaddr_in addr;
>  
>  	DEBUG_CALL("sosendto");
> -	DEBUG_ARG("so = %lx", (long)so);
> -	DEBUG_ARG("m = %lx", (long)m);
> +	DEBUG_ARG("so = %p", so);
> +	DEBUG_ARG("m = %p", m);
>  
>          addr.sin_family = AF_INET;
>  	if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) ==
> diff --git a/slirp/tcp_input.c b/slirp/tcp_input.c
> index f946db8..1cd0735 100644
> --- a/slirp/tcp_input.c
> +++ b/slirp/tcp_input.c
> @@ -231,8 +231,8 @@ tcp_input(struct mbuf *m, int iphlen, struct socket *inso)
>      Slirp *slirp;
>  
>  	DEBUG_CALL("tcp_input");
> -	DEBUG_ARGS((dfd, " m = %8lx  iphlen = %2d  inso = %lx\n",
> -		    (long )m, iphlen, (long )inso ));
> +	DEBUG_ARGS((dfd, " m = %p  iphlen = %2d  inso = %p\n",
> +		    m, iphlen, inso));
>  
>  	/*
>  	 * If called with m == 0, then we're continuing the connect
> @@ -917,8 +917,8 @@ trimthenstep6:
>  
>  		if (SEQ_LEQ(ti->ti_ack, tp->snd_una)) {
>  			if (ti->ti_len == 0 && tiwin == tp->snd_wnd) {
> -			  DEBUG_MISC((dfd, " dup ack  m = %lx  so = %lx\n",
> -				      (long )m, (long )so));
> +			  DEBUG_MISC((dfd, " dup ack  m = %p  so = %p\n",
> +				      m, so));
>  				/*
>  				 * If we have outstanding data (other than
>  				 * a window probe), this is a completely
> @@ -1296,7 +1296,7 @@ tcp_dooptions(struct tcpcb *tp, u_char *cp, int cnt, struct tcpiphdr *ti)
>  	int opt, optlen;
>  
>  	DEBUG_CALL("tcp_dooptions");
> -	DEBUG_ARGS((dfd, " tp = %lx  cnt=%i\n", (long)tp, cnt));
> +	DEBUG_ARGS((dfd, " tp = %p  cnt=%i\n", tp, cnt));
>  
>  	for (; cnt > 0; cnt -= optlen, cp += optlen) {
>  		opt = cp[0];
> @@ -1377,7 +1377,7 @@ tcp_xmit_timer(register struct tcpcb *tp, int rtt)
>  	register short delta;
>  
>  	DEBUG_CALL("tcp_xmit_timer");
> -	DEBUG_ARG("tp = %lx", (long)tp);
> +	DEBUG_ARG("tp = %p", tp);
>  	DEBUG_ARG("rtt = %d", rtt);
>  
>  	if (tp->t_srtt != 0) {
> @@ -1465,7 +1465,7 @@ tcp_mss(struct tcpcb *tp, u_int offer)
>  	int mss;
>  
>  	DEBUG_CALL("tcp_mss");
> -	DEBUG_ARG("tp = %lx", (long)tp);
> +	DEBUG_ARG("tp = %p", tp);
>  	DEBUG_ARG("offer = %d", offer);
>  
>  	mss = min(IF_MTU, IF_MRU) - sizeof(struct tcpiphdr);
> diff --git a/slirp/tcp_output.c b/slirp/tcp_output.c
> index 8aa3d90..fafca58 100644
> --- a/slirp/tcp_output.c
> +++ b/slirp/tcp_output.c
> @@ -66,7 +66,7 @@ tcp_output(struct tcpcb *tp)
>  	int idle, sendalot;
>  
>  	DEBUG_CALL("tcp_output");
> -	DEBUG_ARG("tp = %lx", (long )tp);
> +	DEBUG_ARG("tp = %p", tp);
>  
>  	/*
>  	 * Determine length of data that should be transmitted,
> diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
> index 7571c5a..e161ed2 100644
> --- a/slirp/tcp_subr.c
> +++ b/slirp/tcp_subr.c
> @@ -224,7 +224,7 @@ tcp_newtcpcb(struct socket *so)
>  struct tcpcb *tcp_drop(struct tcpcb *tp, int err)
>  {
>  	DEBUG_CALL("tcp_drop");
> -	DEBUG_ARG("tp = %lx", (long)tp);
> +	DEBUG_ARG("tp = %p", tp);
>  	DEBUG_ARG("errno = %d", errno);
>  
>  	if (TCPS_HAVERCVDSYN(tp->t_state)) {
> @@ -249,7 +249,7 @@ tcp_close(struct tcpcb *tp)
>  	register struct mbuf *m;
>  
>  	DEBUG_CALL("tcp_close");
> -	DEBUG_ARG("tp = %lx", (long )tp);
> +	DEBUG_ARG("tp = %p", tp);
>  
>  	/* free the reassembly queue, if any */
>  	t = tcpfrag_list_first(tp);
> @@ -290,7 +290,7 @@ tcp_sockclosed(struct tcpcb *tp)
>  {
>  
>  	DEBUG_CALL("tcp_sockclosed");
> -	DEBUG_ARG("tp = %lx", (long)tp);
> +	DEBUG_ARG("tp = %p", tp);
>  
>  	switch (tp->t_state) {
>  
> @@ -330,7 +330,7 @@ int tcp_fconnect(struct socket *so)
>    int ret=0;
>  
>    DEBUG_CALL("tcp_fconnect");
> -  DEBUG_ARG("so = %lx", (long )so);
> +  DEBUG_ARG("so = %p", so);
>  
>    if( (ret = so->s = qemu_socket(AF_INET,SOCK_STREAM,0)) >= 0) {
>      int opt, s=so->s;
> @@ -393,7 +393,7 @@ void tcp_connect(struct socket *inso)
>      int s, opt;
>  
>      DEBUG_CALL("tcp_connect");
> -    DEBUG_ARG("inso = %lx", (long)inso);
> +    DEBUG_ARG("inso = %p", inso);
>  
>      /*
>       * If it's an SS_ACCEPTONCE socket, no need to socreate()
> @@ -564,8 +564,8 @@ tcp_emu(struct socket *so, struct mbuf *m)
>  	char *bptr;
>  
>  	DEBUG_CALL("tcp_emu");
> -	DEBUG_ARG("so = %lx", (long)so);
> -	DEBUG_ARG("m = %lx", (long)m);
> +	DEBUG_ARG("so = %p", so);
> +	DEBUG_ARG("m = %p", m);
>  
>  	switch(so->so_emu) {
>  		int x, i;
> @@ -900,7 +900,7 @@ int tcp_ctl(struct socket *so)
>      int do_pty;
>  
>      DEBUG_CALL("tcp_ctl");
> -    DEBUG_ARG("so = %lx", (long )so);
> +    DEBUG_ARG("so = %p", so);
>  
>      if (so->so_faddr.s_addr != slirp->vhost_addr.s_addr) {
>          /* Check if it's pty_exec */
> diff --git a/slirp/udp.c b/slirp/udp.c
> index f77e00f..fee13b4 100644
> --- a/slirp/udp.c
> +++ b/slirp/udp.c
> @@ -72,7 +72,7 @@ udp_input(register struct mbuf *m, int iphlen)
>  	struct socket *so;
>  
>  	DEBUG_CALL("udp_input");
> -	DEBUG_ARG("m = %lx", (long)m);
> +	DEBUG_ARG("m = %p", m);
>  	DEBUG_ARG("iphlen = %d", iphlen);
>  
>  	/*
> @@ -241,8 +241,8 @@ int udp_output2(struct socket *so, struct mbuf *m,
>  	int error = 0;
>  
>  	DEBUG_CALL("udp_output");
> -	DEBUG_ARG("so = %lx", (long)so);
> -	DEBUG_ARG("m = %lx", (long)m);
> +	DEBUG_ARG("so = %p", so);
> +	DEBUG_ARG("m = %p", m);
>  	DEBUG_ARG("saddr = %lx", (long)saddr->sin_addr.s_addr);
>  	DEBUG_ARG("daddr = %lx", (long)daddr->sin_addr.s_addr);
>  
> 

  parent reply	other threads:[~2015-10-27 18:40 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-29  7:12 [Qemu-devel] [PATCH] slirp: Fix type casts and format strings in debug code Stefan Weil
2015-09-25 19:48 ` Stefan Weil
2015-10-27 18:09   ` Stefan Weil
2015-10-27 18:39 ` Paolo Bonzini [this message]
2015-10-28  3:16   ` Jason Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=562FC4FA.50600@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=jan.kiszka@siemens.com \
    --cc=jasowang@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=sw@weilnetz.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).