| Message ID | 1480108791-16178-1-git-send-email-yuval.shaia@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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.
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)
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
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))
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(-)