All of lore.kernel.org
 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 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.