From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52192) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bty79-0004Lf-3f for qemu-devel@nongnu.org; Tue, 11 Oct 2016 10:33:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bty72-0002RQ-5x for qemu-devel@nongnu.org; Tue, 11 Oct 2016 10:33:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43322) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bty71-0002RE-St for qemu-devel@nongnu.org; Tue, 11 Oct 2016 10:33:00 -0400 References: <1476171207-18472-1-git-send-email-zhangchen.fnst@cn.fujitsu.com> From: Eric Blake Message-ID: <8cc76dd2-5c9d-a96c-9da9-c775cbdfc09e@redhat.com> Date: Tue, 11 Oct 2016 09:32:58 -0500 MIME-Version: 1.0 In-Reply-To: <1476171207-18472-1-git-send-email-zhangchen.fnst@cn.fujitsu.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="nhoxC00K3drcP91nAxQ5MPqFbqSvR9I5R" Subject: Re: [Qemu-devel] [PATCH V2] colo-proxy: fix memory leak List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zhang Chen , qemu devel , Jason Wang Cc: Li Zhijian , Paolo Bonzini This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --nhoxC00K3drcP91nAxQ5MPqFbqSvR9I5R From: Eric Blake To: Zhang Chen , qemu devel , Jason Wang Cc: Li Zhijian , Paolo Bonzini Message-ID: <8cc76dd2-5c9d-a96c-9da9-c775cbdfc09e@redhat.com> Subject: Re: [PATCH V2] colo-proxy: fix memory leak References: <1476171207-18472-1-git-send-email-zhangchen.fnst@cn.fujitsu.com> In-Reply-To: <1476171207-18472-1-git-send-email-zhangchen.fnst@cn.fujitsu.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/11/2016 02:33 AM, Zhang Chen wrote: > Fix memory leak in colo-compare.c and filter-rewriter.c > Report by Coverity. This part is fine. >=20 > v2: > - use traces instead of fprintf in colo-compare.c >=20 > v1: > - initial patch =2E..but this part should live... >=20 > Signed-off-by: Zhang Chen > --- =2E..here, after the --- separator, where 'git am' will ignore it. It is= useful information to list readers, but will make no sense in qemu.git history a year from now (we don't care how many versions it took to get to the version that was committed). The maintainer can fix it, so by itself, that's not a reason to send a v3. > net/colo-compare.c | 11 +++-------- > net/filter-rewriter.c | 17 +++++------------ > trace-events | 1 + > 3 files changed, 9 insertions(+), 20 deletions(-) >=20 > @@ -219,11 +218,9 @@ static int colo_packet_compare_tcp(Packet *spkt, P= acket *ppkt) > (spkt->size - ETH_HLEN)); > =20 > if (res !=3D 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMP= ARE)) { > - sdebug =3D strdup(inet_ntoa(ppkt->ip->ip_src)); > - ddebug =3D strdup(inet_ntoa(ppkt->ip->ip_dst)); > - fprintf(stderr, "%s: src/dst: %s/%s p: seq/ack=3D%u/%u" > - " s: seq/ack=3D%u/%u res=3D%d flags=3D%x/%x\n", > - __func__, sdebug, ddebug, > + trace_colo_compare_pkt_info(__func__, net/filter-rewriter.c is the only file that currently uses trace_...(__func__), that's because the trace mechanism itself already expands the trace_* call to something with enough context that manually adding __func__ just gives redundant information. I'd rather see __func__ removed from the trace call. > + inet_ntoa(ppkt->ip->ip_src), > + inet_ntoa(ppkt->ip->ip_dst), > (unsigned int)ntohl(ptcp->th_seq), > (unsigned int)ntohl(ptcp->th_ack), > (unsigned int)ntohl(stcp->th_seq), Are these casts still needed? > +++ b/trace-events > @@ -149,6 +149,7 @@ colo_compare_icmp_miscompare(const char *sta, int s= ize) ": %s =3D %d" > colo_compare_ip_info(int psize, const char *sta, const char *stb, int = ssize, const char *stc, const char *std) "ppkt size =3D %d, ip_src =3D %s= , ip_dst =3D %s, spkt size =3D %d, ip_src =3D %s, ip_dst =3D %s" > colo_old_packet_check_found(int64_t old_time) "%" PRId64 > colo_compare_miscompare(void) "" > +colo_compare_pkt_info(const char *func, const char *src, const char *d= st, uint32_t pseq, uint32_t pack, uint32_t sseq, uint32_t sack, int res, = uint32_t pflag, uint32_t sflag) "%s: src/dst: %s/%s p: seq/ack=3D%u/%u = s: seq/ack=3D%u/%u res=3D%d flags=3D%x/%x\n" Again, the 'const char *func' portion is not needed. Looking forward to v3. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --nhoxC00K3drcP91nAxQ5MPqFbqSvR9I5R Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJX/PgaAAoJEKeha0olJ0NqM40IAKq31k3nL5m8aWYOl5cXzJQm JSPabcUpywXBPlCl85fZUwRAdmSfyCUmrrWRLIq3cs7OQ3tpuzBHLYMfkhpRm9y6 0dTOlZ/ujOrkzmNE5u7c7YFj+tCAkZXdAUiHNKt7nYgQrp+am8bh78h+waENmZ1/ 31izwdxjBkWQz4qL28d/OIF1YVAggvy1c++g3XFzo60pe1RD9JSrhpv6XFf/RyoP mmz97wO8FXaUcMdpZuOdG33UfsWPytokZW2W1QgSBLT7kWN/BTFcNOgqBnQRrARC hmXeOsc6Gw7djJYyzev7Rnj1GXgUpP1JgIPFRoKpHHlDO0EH2zYTVxtknfSJ1mQ= =+pt8 -----END PGP SIGNATURE----- --nhoxC00K3drcP91nAxQ5MPqFbqSvR9I5R--