* [Qemu-devel] [PATCH] {disas, slirp}: Replace min/max with MIN/MAX macros
@ 2016-11-25 21:19 Yuval Shaia
2016-11-28 7:15 ` Fam Zheng
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Yuval Shaia @ 2016-11-25 21:19 UTC (permalink / raw)
To: peter.maydell, qemu-devel, samuel.thibault, jan.kiszka, armbru
Cc: yuval.shaia
Use generic declaration of min and max macros instead of private ones.
Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com>
---
disas/m68k.c | 4 ----
slirp/dhcpv6.c | 2 +-
slirp/ip6_icmp.c | 2 +-
slirp/slirp.c | 2 +-
slirp/slirp.h | 5 -----
slirp/tcp_input.c | 14 +++++++-------
slirp/tcp_output.c | 6 +++---
slirp/tcp_timer.c | 2 +-
slirp/tcpip.h | 4 +++-
9 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/disas/m68k.c b/disas/m68k.c
index 8e7c3f7..ca57248 100644
--- a/disas/m68k.c
+++ b/disas/m68k.c
@@ -4698,10 +4698,6 @@ get_field (const unsigned char *data, enum floatformat_byteorders order,
return result;
}
-#ifndef min
-#define min(a, b) ((a) < (b) ? (a) : (b))
-#endif
-
/* Convert from FMT to a double.
FROM is the address of the extended float.
Store the double in *TO. */
diff --git a/slirp/dhcpv6.c b/slirp/dhcpv6.c
index 02c51c7..d266611 100644
--- a/slirp/dhcpv6.c
+++ b/slirp/dhcpv6.c
@@ -168,7 +168,7 @@ static void dhcpv6_info_request(Slirp *slirp, struct sockaddr_in6 *srcsas,
sa[0], sa[1], sa[2], sa[3], sa[4], sa[5], sa[6], sa[7],
sa[8], sa[9], sa[10], sa[11], sa[12], sa[13], sa[14],
sa[15], slirp->bootp_filename);
- slen = min(slen, smaxlen);
+ slen = MIN(slen, smaxlen);
*resp++ = slen >> 8; /* option-len high byte */
*resp++ = slen; /* option-len low byte */
resp += slen;
diff --git a/slirp/ip6_icmp.c b/slirp/ip6_icmp.c
index 6d18e28..298a48d 100644
--- a/slirp/ip6_icmp.c
+++ b/slirp/ip6_icmp.c
@@ -95,7 +95,7 @@ void icmp6_send_error(struct mbuf *m, uint8_t type, uint8_t code)
#endif
rip->ip_nh = IPPROTO_ICMPV6;
- const int error_data_len = min(m->m_len,
+ const int error_data_len = MIN(m->m_len,
IF_MTU - (sizeof(struct ip6) + ICMP6_ERROR_MINLEN));
rip->ip_pl = htons(ICMP6_ERROR_MINLEN + error_data_len);
t->m_len = sizeof(struct ip6) + ntohs(rip->ip_pl);
diff --git a/slirp/slirp.c b/slirp/slirp.c
index d67eda1..3987cae 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -774,7 +774,7 @@ void slirp_pollfds_poll(GArray *pollfds, int select_error)
static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
{
struct slirp_arphdr *ah = (struct slirp_arphdr *)(pkt + ETH_HLEN);
- uint8_t arp_reply[max(ETH_HLEN + sizeof(struct slirp_arphdr), 64)];
+ uint8_t arp_reply[MAX(ETH_HLEN + sizeof(struct slirp_arphdr), 64)];
struct ethhdr *reh = (struct ethhdr *)arp_reply;
struct slirp_arphdr *rah = (struct slirp_arphdr *)(arp_reply + ETH_HLEN);
int ar_op;
diff --git a/slirp/slirp.h b/slirp/slirp.h
index a1f3139..3877f66 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -292,9 +292,4 @@ int tcp_emu(struct socket *, struct mbuf *);
int tcp_ctl(struct socket *);
struct tcpcb *tcp_drop(struct tcpcb *tp, int err);
-#ifndef _WIN32
-#define min(x,y) ((x) < (y) ? (x) : (y))
-#define max(x,y) ((x) > (y) ? (x) : (y))
-#endif
-
#endif
diff --git a/slirp/tcp_input.c b/slirp/tcp_input.c
index c5063a9..42c7f2f 100644
--- a/slirp/tcp_input.c
+++ b/slirp/tcp_input.c
@@ -596,7 +596,7 @@ findso:
win = sbspace(&so->so_rcv);
if (win < 0)
win = 0;
- tp->rcv_wnd = max(win, (int)(tp->rcv_adv - tp->rcv_nxt));
+ tp->rcv_wnd = MAX(win, (int)(tp->rcv_adv - tp->rcv_nxt));
}
switch (tp->t_state) {
@@ -1065,7 +1065,7 @@ trimthenstep6:
else if (++tp->t_dupacks == TCPREXMTTHRESH) {
tcp_seq onxt = tp->snd_nxt;
u_int win =
- min(tp->snd_wnd, tp->snd_cwnd) / 2 /
+ MIN(tp->snd_wnd, tp->snd_cwnd) / 2 /
tp->t_maxseg;
if (win < 2)
@@ -1138,7 +1138,7 @@ trimthenstep6:
if (cw > tp->snd_ssthresh)
incr = incr * incr / cw;
- tp->snd_cwnd = min(cw + incr, TCP_MAXWIN<<tp->snd_scale);
+ tp->snd_cwnd = MIN(cw + incr, TCP_MAXWIN<<tp->snd_scale);
}
if (acked > so->so_snd.sb_cc) {
tp->snd_wnd -= so->so_snd.sb_cc;
@@ -1586,11 +1586,11 @@ tcp_mss(struct tcpcb *tp, u_int offer)
switch (so->so_ffamily) {
case AF_INET:
- mss = min(IF_MTU, IF_MRU) - sizeof(struct tcphdr)
+ mss = MIN(IF_MTU, IF_MRU) - sizeof(struct tcphdr)
+ sizeof(struct ip);
break;
case AF_INET6:
- mss = min(IF_MTU, IF_MRU) - sizeof(struct tcphdr)
+ mss = MIN(IF_MTU, IF_MRU) - sizeof(struct tcphdr)
+ sizeof(struct ip6);
break;
default:
@@ -1598,8 +1598,8 @@ tcp_mss(struct tcpcb *tp, u_int offer)
}
if (offer)
- mss = min(mss, offer);
- mss = max(mss, 32);
+ mss = MIN(mss, offer);
+ mss = MAX(mss, 32);
if (mss < tp->t_maxseg || offer != 0)
tp->t_maxseg = mss;
diff --git a/slirp/tcp_output.c b/slirp/tcp_output.c
index 819db27..263af82 100644
--- a/slirp/tcp_output.c
+++ b/slirp/tcp_output.c
@@ -88,7 +88,7 @@ tcp_output(struct tcpcb *tp)
again:
sendalot = 0;
off = tp->snd_nxt - tp->snd_una;
- win = min(tp->snd_wnd, tp->snd_cwnd);
+ win = MIN(tp->snd_wnd, tp->snd_cwnd);
flags = tcp_outflags[tp->t_state];
@@ -127,7 +127,7 @@ again:
}
}
- len = min(so->so_snd.sb_cc, win) - off;
+ len = MIN(so->so_snd.sb_cc, win) - off;
if (len < 0) {
/*
@@ -193,7 +193,7 @@ again:
* taking into account that we are limited by
* TCP_MAXWIN << tp->rcv_scale.
*/
- long adv = min(win, (long)TCP_MAXWIN << tp->rcv_scale) -
+ long adv = MIN(win, (long)TCP_MAXWIN << tp->rcv_scale) -
(tp->rcv_adv - tp->rcv_nxt);
if (adv >= (long) (2 * tp->t_maxseg))
diff --git a/slirp/tcp_timer.c b/slirp/tcp_timer.c
index f9060c7..6a091b8 100644
--- a/slirp/tcp_timer.c
+++ b/slirp/tcp_timer.c
@@ -233,7 +233,7 @@ tcp_timers(register struct tcpcb *tp, int timer)
* to go below this.)
*/
{
- u_int win = min(tp->snd_wnd, tp->snd_cwnd) / 2 / tp->t_maxseg;
+ u_int win = MIN(tp->snd_wnd, tp->snd_cwnd) / 2 / tp->t_maxseg;
if (win < 2)
win = 2;
tp->snd_cwnd = tp->t_maxseg;
diff --git a/slirp/tcpip.h b/slirp/tcpip.h
index 7bdb971..71eb6a6 100644
--- a/slirp/tcpip.h
+++ b/slirp/tcpip.h
@@ -30,6 +30,8 @@
* tcpip.h,v 1.3 1994/08/21 05:27:40 paul Exp
*/
+#include "qemu/osdep.h"
+
#ifndef TCPIP_H
#define TCPIP_H
@@ -85,7 +87,7 @@ struct tcpiphdr {
/* This is the difference between the size of a tcpiphdr structure, and the
* size of actual ip+tcp headers, rounded up since we need to align data. */
#define TCPIPHDR_DELTA\
- (max(0,\
+ (MAX(0,\
(sizeof(struct tcpiphdr)\
- sizeof(struct ip) - sizeof(struct tcphdr) + 3) & ~3))
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] {disas, slirp}: Replace min/max with MIN/MAX macros
2016-11-25 21:19 [Qemu-devel] [PATCH] {disas, slirp}: Replace min/max with MIN/MAX macros Yuval Shaia
@ 2016-11-28 7:15 ` Fam Zheng
2016-11-28 7:39 ` Markus Armbruster
2016-11-29 13:32 ` no-reply
2 siblings, 0 replies; 5+ messages in thread
From: Fam Zheng @ 2016-11-28 7:15 UTC (permalink / raw)
To: Yuval Shaia
Cc: peter.maydell, qemu-devel, samuel.thibault, jan.kiszka, armbru
On Fri, 11/25 23:19, Yuval Shaia wrote:
> diff --git a/slirp/tcpip.h b/slirp/tcpip.h
> index 7bdb971..71eb6a6 100644
> --- a/slirp/tcpip.h
> +++ b/slirp/tcpip.h
> @@ -30,6 +30,8 @@
> * tcpip.h,v 1.3 1994/08/21 05:27:40 paul Exp
> */
>
> +#include "qemu/osdep.h"
> +
It's a rule that sources in QEMU always include "osdep.h" first, so this is not
necessary. There is a comment on this in scripts/clean-includes:
...
# .h files will have redundant includes (including includes of osdep.h)
# removed.
...
Other headers that uses MAX() don't include it either.
> #ifndef TCPIP_H
> #define TCPIP_H
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] {disas, slirp}: Replace min/max with MIN/MAX macros
2016-11-25 21:19 [Qemu-devel] [PATCH] {disas, slirp}: Replace min/max with MIN/MAX macros Yuval Shaia
2016-11-28 7:15 ` Fam Zheng
@ 2016-11-28 7:39 ` Markus Armbruster
2016-11-28 9:14 ` Yuval Shaia
2016-11-29 13:32 ` no-reply
2 siblings, 1 reply; 5+ messages in thread
From: Markus Armbruster @ 2016-11-28 7:39 UTC (permalink / raw)
To: Yuval Shaia; +Cc: peter.maydell, qemu-devel, samuel.thibault, jan.kiszka
The "{disas, slirp}: " prefix is unusual. Better: "disas, slirp: ".
But I'd instead split the patch into the slirp part, where you really
replace stuff, and the disas part, where you merely drop an unused macro
definition.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] {disas, slirp}: Replace min/max with MIN/MAX macros
2016-11-28 7:39 ` Markus Armbruster
@ 2016-11-28 9:14 ` Yuval Shaia
0 siblings, 0 replies; 5+ messages in thread
From: Yuval Shaia @ 2016-11-28 9:14 UTC (permalink / raw)
To: Markus Armbruster; +Cc: peter.maydell, qemu-devel, samuel.thibault, jan.kiszka
On Mon, Nov 28, 2016 at 08:39:21AM +0100, Markus Armbruster wrote:
> The "{disas, slirp}: " prefix is unusual. Better: "disas, slirp: ".
> But I'd instead split the patch into the slirp part, where you really
> replace stuff, and the disas part, where you merely drop an unused macro
> definition.
Thanks,
Accepting your first suggestion as just now realized that actually this
macro is in use (disas/m68k.c lines 4732 and 4796).
(Can't explain how i missed that in v0)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] {disas, slirp}: Replace min/max with MIN/MAX macros
2016-11-25 21:19 [Qemu-devel] [PATCH] {disas, slirp}: Replace min/max with MIN/MAX macros Yuval Shaia
2016-11-28 7:15 ` Fam Zheng
2016-11-28 7:39 ` Markus Armbruster
@ 2016-11-29 13:32 ` no-reply
2 siblings, 0 replies; 5+ messages in thread
From: no-reply @ 2016-11-29 13:32 UTC (permalink / raw)
To: yuval.shaia
Cc: famz, peter.maydell, qemu-devel, samuel.thibault, jan.kiszka, armbru
Hi,
Your series seems to have some coding style problems. See output below for
more information:
Subject: [Qemu-devel] [PATCH] {disas, slirp}: Replace min/max with MIN/MAX macros
Type: series
Message-id: 1480108791-16178-1-git-send-email-yuval.shaia@oracle.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
7eb457b {disas, slirp}: Replace min/max with MIN/MAX macros
=== OUTPUT BEGIN ===
Checking PATCH 1/1: {disas, slirp}: Replace min/max with MIN/MAX macros...
ERROR: code indent should never use tabs
#88: FILE: slirp/tcp_input.c:599:
+^I tp->rcv_wnd = MAX(win, (int)(tp->rcv_adv - tp->rcv_nxt));$
ERROR: code indent should never use tabs
#97: FILE: slirp/tcp_input.c:1068:
+^I^I^I^I^I MIN(tp->snd_wnd, tp->snd_cwnd) / 2 /$
ERROR: code indent should never use tabs
#106: FILE: slirp/tcp_input.c:1141:
+^I^I tp->snd_cwnd = MIN(cw + incr, TCP_MAXWIN<<tp->snd_scale);$
ERROR: spaces required around that '<<' (ctx:VxV)
#106: FILE: slirp/tcp_input.c:1141:
+ tp->snd_cwnd = MIN(cw + incr, TCP_MAXWIN<<tp->snd_scale);
^
ERROR: code indent should never use tabs
#115: FILE: slirp/tcp_input.c:1589:
+^I mss = MIN(IF_MTU, IF_MRU) - sizeof(struct tcphdr)$
ERROR: code indent should never use tabs
#120: FILE: slirp/tcp_input.c:1593:
+^I mss = MIN(IF_MTU, IF_MRU) - sizeof(struct tcphdr)$
ERROR: code indent should never use tabs
#130: FILE: slirp/tcp_input.c:1601:
+^I^Imss = MIN(mss, offer);$
ERROR: code indent should never use tabs
#131: FILE: slirp/tcp_input.c:1602:
+^Imss = MAX(mss, 32);$
ERROR: code indent should never use tabs
#144: FILE: slirp/tcp_output.c:91:
+^Iwin = MIN(tp->snd_wnd, tp->snd_cwnd);$
ERROR: code indent should never use tabs
#153: FILE: slirp/tcp_output.c:130:
+^Ilen = MIN(so->so_snd.sb_cc, win) - off;$
ERROR: code indent should never use tabs
#162: FILE: slirp/tcp_output.c:196:
+^I^Ilong adv = MIN(win, (long)TCP_MAXWIN << tp->rcv_scale) -$
ERROR: code indent should never use tabs
#175: FILE: slirp/tcp_timer.c:236:
+^I^Iu_int win = MIN(tp->snd_wnd, tp->snd_cwnd) / 2 / tp->t_maxseg;$
total: 12 errors, 0 warnings, 138 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-11-29 13:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-25 21:19 [Qemu-devel] [PATCH] {disas, slirp}: Replace min/max with MIN/MAX macros Yuval Shaia
2016-11-28 7:15 ` Fam Zheng
2016-11-28 7:39 ` Markus Armbruster
2016-11-28 9:14 ` Yuval Shaia
2016-11-29 13:32 ` no-reply
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.