[{"id":1769936,"web_url":"http://patchwork.ozlabs.org/comment/1769936/","msgid":"<20170918075504.gdx4q7je5uneefgg@unicorn.suse.cz>","list_archive_url":null,"date":"2017-09-18T07:55:05","subject":"Re: [PATCHv2 iproute2 1/2] lib/libnetlink: re malloc buff if size is\n\tnot enough","submitter":{"id":11892,"url":"http://patchwork.ozlabs.org/api/people/11892/","name":"Michal Kubecek","email":"mkubecek@suse.cz"},"content":"On Wed, Sep 13, 2017 at 05:59:39PM +0800, Hangbin Liu wrote:\n> With commit 72b365e8e0fd (\"libnetlink: Double the dump buffer size\")\n> we doubled the buffer size to support more VFs. But the VFs number is\n> increasing all the time. Some customers even use more than 200 VFs now.\n> \n> We could not double it everytime when the buffer is not enough. Let's just\n> not hard code the buffer size and malloc the correct number when running.\n> \n> Introduce function rtnl_recvmsg() to always return a newly allocated buffer.\n> The caller need to free it after using.\n> \n> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>\n> Signed-off-by: Phil Sutter <phil@nwl.cc>\n> ---\n>  lib/libnetlink.c | 112 ++++++++++++++++++++++++++++++++++++++-----------------\n>  1 file changed, 78 insertions(+), 34 deletions(-)\n> \n> diff --git a/lib/libnetlink.c b/lib/libnetlink.c\n> index be7ac86..e3fa7cf 100644\n> --- a/lib/libnetlink.c\n> +++ b/lib/libnetlink.c\n> @@ -402,6 +402,62 @@ static void rtnl_dump_error(const struct rtnl_handle *rth,\n>  \t}\n>  }\n>  \n> +static int rtnl_recvmsg(int fd, struct msghdr *msg, char **answer)\n> +{\n> +\tstruct iovec *iov;\n> +\tint len = -1, buf_len = 32768;\n> +\tchar *bufp, *buf = NULL;\n> +\n> +\tint flag = MSG_PEEK | MSG_TRUNC;\n> +\n> +realloc:\n> +\tbufp = realloc(buf, buf_len);\n> +\n> +\tif (bufp == NULL) {\n> +\t\tfprintf(stderr, \"malloc error: not enough buffer\\n\");\n> +\t\tfree(buf);\n> +\t\treturn -ENOMEM;\n> +\t}\n> +\tbuf = bufp;\n> +\tiov = msg->msg_iov;\n> +\tiov->iov_base = buf;\n> +\tiov->iov_len = buf_len;\n> +\n> +recv:\n> +\tlen = recvmsg(fd, msg, flag);\n> +\n> +\tif (len < 0) {\n> +\t\tif (errno == EINTR || errno == EAGAIN)\n> +\t\t\tgoto recv;\n> +\t\tfprintf(stderr, \"netlink receive error %s (%d)\\n\",\n> +\t\t\tstrerror(errno), errno);\n\nfree(buf);\n\n> +\t\treturn len;\n\nMaybe we should return -errno (saved before calling fprintf()) to be\nconsistent.\n\n> +\t}\n> +\n> +\tif (len == 0) {\n> +\t\tfprintf(stderr, \"EOF on netlink\\n\");\n\nfree(buf);\n\n> +\t\treturn -ENODATA;\n> +\t}\n> +\n> +\tif (len > buf_len) {\n> +\t\tbuf_len = len;\n> +\t\tflag = 0;\n> +\t\tgoto realloc;\n> +\t}\n> +\n> +\tif (flag != 0) {\n> +\t\tflag = 0;\n> +\t\tgoto recv;\n> +\t}\n\nThis means that even if the default buffer size is sufficient (which\nshould be most of the time) we make the kernel copy the message to\nuserspace again. Perhaps we could just call recvmsg() with zero length\nto discard the message from the queue in this case. But it's not really\na big problem, I guess.\n\n> +\n> +\tif (answer)\n> +\t\t*answer = buf;\n> +\telse\n> +\t\tfree(buf);\n> +\n> +\treturn len;\n> +}\n> +\n>  int rtnl_dump_filter_l(struct rtnl_handle *rth,\n>  \t\t       const struct rtnl_dump_filter_arg *arg)\n>  {\n> @@ -413,31 +469,18 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,\n>  \t\t.msg_iov = &iov,\n>  \t\t.msg_iovlen = 1,\n>  \t};\n> -\tchar buf[32768];\n> +\tchar *buf;\n>  \tint dump_intr = 0;\n>  \n> -\tiov.iov_base = buf;\n>  \twhile (1) {\n>  \t\tint status;\n>  \t\tconst struct rtnl_dump_filter_arg *a;\n>  \t\tint found_done = 0;\n>  \t\tint msglen = 0;\n>  \n> -\t\tiov.iov_len = sizeof(buf);\n> -\t\tstatus = recvmsg(rth->fd, &msg, 0);\n> -\n> -\t\tif (status < 0) {\n> -\t\t\tif (errno == EINTR || errno == EAGAIN)\n> -\t\t\t\tcontinue;\n> -\t\t\tfprintf(stderr, \"netlink receive error %s (%d)\\n\",\n> -\t\t\t\tstrerror(errno), errno);\n> -\t\t\treturn -1;\n> -\t\t}\n> -\n> -\t\tif (status == 0) {\n> -\t\t\tfprintf(stderr, \"EOF on netlink\\n\");\n> -\t\t\treturn -1;\n> -\t\t}\n> +\t\tstatus = rtnl_recvmsg(rth->fd, &msg, &buf);\n> +\t\tif (status < 0)\n> +\t\t\treturn status;\n>  \n>  \t\tif (rth->dump_fp)\n>  \t\t\tfwrite(buf, 1, NLMSG_ALIGN(status), rth->dump_fp);\n> @@ -462,8 +505,10 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,\n>  \n>  \t\t\t\tif (h->nlmsg_type == NLMSG_DONE) {\n>  \t\t\t\t\terr = rtnl_dump_done(h);\n> -\t\t\t\t\tif (err < 0)\n> +\t\t\t\t\tif (err < 0) {\n> +\t\t\t\t\t\tfree(buf);\n>  \t\t\t\t\t\treturn -1;\n> +\t\t\t\t\t}\n>  \n>  \t\t\t\t\tfound_done = 1;\n>  \t\t\t\t\tbreak; /* process next filter */\n> @@ -471,19 +516,23 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,\n>  \n>  \t\t\t\tif (h->nlmsg_type == NLMSG_ERROR) {\n>  \t\t\t\t\trtnl_dump_error(rth, h);\n> +\t\t\t\t\tfree(buf);\n>  \t\t\t\t\treturn -1;\n>  \t\t\t\t}\n>  \n>  \t\t\t\tif (!rth->dump_fp) {\n>  \t\t\t\t\terr = a->filter(&nladdr, h, a->arg1);\n> -\t\t\t\t\tif (err < 0)\n> +\t\t\t\t\tif (err < 0) {\n> +\t\t\t\t\t\tfree(buf);\n>  \t\t\t\t\t\treturn err;\n> +\t\t\t\t\t}\n>  \t\t\t\t}\n>  \n>  skip_it:\n>  \t\t\t\th = NLMSG_NEXT(h, msglen);\n>  \t\t\t}\n>  \t\t}\n> +\t\tfree(buf);\n\nWe only free the last buffer returned by rtnl_recvmsg() this way. IMHO\nthis free(buf) should be moved inside the loop.\n\n>  \n>  \t\tif (found_done) {\n>  \t\t\tif (dump_intr)\n> @@ -543,7 +592,7 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,\n>  \t\t.msg_iov = &iov,\n>  \t\t.msg_iovlen = 1,\n>  \t};\n> -\tchar   buf[32768] = {};\n> +\tchar *buf;\n>  \n>  \tn->nlmsg_seq = seq = ++rtnl->seq;\n>  \n> @@ -556,22 +605,12 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,\n>  \t\treturn -1;\n>  \t}\n>  \n> -\tiov.iov_base = buf;\n>  \twhile (1) {\n> -\t\tiov.iov_len = sizeof(buf);\n> -\t\tstatus = recvmsg(rtnl->fd, &msg, 0);\n> +\t\tstatus = rtnl_recvmsg(rtnl->fd, &msg, &buf);\n> +\n> +\t\tif (status < 0)\n> +\t\t\treturn status;\n>  \n> -\t\tif (status < 0) {\n> -\t\t\tif (errno == EINTR || errno == EAGAIN)\n> -\t\t\t\tcontinue;\n> -\t\t\tfprintf(stderr, \"netlink receive error %s (%d)\\n\",\n> -\t\t\t\tstrerror(errno), errno);\n> -\t\t\treturn -1;\n> -\t\t}\n> -\t\tif (status == 0) {\n> -\t\t\tfprintf(stderr, \"EOF on netlink\\n\");\n> -\t\t\treturn -1;\n> -\t\t}\n>  \t\tif (msg.msg_namelen != sizeof(nladdr)) {\n>  \t\t\tfprintf(stderr,\n>  \t\t\t\t\"sender address length == %d\\n\",\n> @@ -585,6 +624,7 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,\n>  \t\t\tif (l < 0 || len > status) {\n>  \t\t\t\tif (msg.msg_flags & MSG_TRUNC) {\n>  \t\t\t\t\tfprintf(stderr, \"Truncated message\\n\");\n> +\t\t\t\t\tfree(buf);\n>  \t\t\t\t\treturn -1;\n>  \t\t\t\t}\n>  \t\t\t\tfprintf(stderr,\n> @@ -611,6 +651,7 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,\n>  \t\t\t\t\tif (answer)\n>  \t\t\t\t\t\tmemcpy(answer, h,\n>  \t\t\t\t\t\t       MIN(maxlen, h->nlmsg_len));\n> +\t\t\t\t\tfree(buf);\n>  \t\t\t\t\treturn 0;\n>  \t\t\t\t}\n>  \n> @@ -619,12 +660,14 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,\n>  \t\t\t\t\trtnl_talk_error(h, err, errfn);\n>  \n>  \t\t\t\terrno = -err->error;\n> +\t\t\t\tfree(buf);\n>  \t\t\t\treturn -1;\n>  \t\t\t}\n>  \n>  \t\t\tif (answer) {\n>  \t\t\t\tmemcpy(answer, h,\n>  \t\t\t\t       MIN(maxlen, h->nlmsg_len));\n> +\t\t\t\tfree(buf);\n>  \t\t\t\treturn 0;\n>  \t\t\t}\n>  \n> @@ -633,6 +676,7 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,\n>  \t\t\tstatus -= NLMSG_ALIGN(len);\n>  \t\t\th = (struct nlmsghdr *)((char *)h + NLMSG_ALIGN(len));\n>  \t\t}\n> +\t\tfree(buf);\n\nSame as above.\n\n>  \n>  \t\tif (msg.msg_flags & MSG_TRUNC) {\n>  \t\t\tfprintf(stderr, \"Message truncated\\n\");\n> -- \n> 2.5.5\n>","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xwdb31g4Kz9s3w\n\tfor <patchwork-incoming@ozlabs.org>;\n\tMon, 18 Sep 2017 17:55:11 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752256AbdIRHzJ (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tMon, 18 Sep 2017 03:55:09 -0400","from mx2.suse.de ([195.135.220.15]:51259 \"EHLO mx1.suse.de\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1752106AbdIRHzI (ORCPT <rfc822;netdev@vger.kernel.org>);\n\tMon, 18 Sep 2017 03:55:08 -0400","from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx1.suse.de (Postfix) with ESMTP id 8DD3CAAB9;\n\tMon, 18 Sep 2017 07:55:06 +0000 (UTC)","by unicorn.suse.cz (Postfix, from userid 1000)\n\tid 82643A0F06; Mon, 18 Sep 2017 09:55:05 +0200 (CEST)"],"X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","Date":"Mon, 18 Sep 2017 09:55:05 +0200","From":"Michal Kubecek <mkubecek@suse.cz>","To":"Hangbin Liu <liuhangbin@gmail.com>","Cc":"netdev@vger.kernel.org, Stephen Hemminger <stephen@networkplumber.org>,\n\tPhil Sutter <phil@nwl.cc>","Subject":"Re: [PATCHv2 iproute2 1/2] lib/libnetlink: re malloc buff if size is\n\tnot enough","Message-ID":"<20170918075504.gdx4q7je5uneefgg@unicorn.suse.cz>","References":"<1505296780-8444-1-git-send-email-liuhangbin@gmail.com>\n\t<1505296780-8444-2-git-send-email-liuhangbin@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<1505296780-8444-2-git-send-email-liuhangbin@gmail.com>","User-Agent":"NeoMutt/20170421 (1.8.2)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1770627,"web_url":"http://patchwork.ozlabs.org/comment/1770627/","msgid":"<20170919030520.GL5465@leo.usersys.redhat.com>","list_archive_url":null,"date":"2017-09-19T03:05:20","subject":"Re: [PATCHv2 iproute2 1/2] lib/libnetlink: re malloc buff if size is\n\tnot enough","submitter":{"id":15639,"url":"http://patchwork.ozlabs.org/api/people/15639/","name":"Hangbin Liu","email":"liuhangbin@gmail.com"},"content":"Hi Michal,\n\nOn Mon, Sep 18, 2017 at 09:55:05AM +0200, Michal Kubecek wrote:\n> > +static int rtnl_recvmsg(int fd, struct msghdr *msg, char **answer)\n> > +{\n> > +\tstruct iovec *iov;\n> > +\tint len = -1, buf_len = 32768;\n> > +\tchar *bufp, *buf = NULL;\n> > +\n> > +\tint flag = MSG_PEEK | MSG_TRUNC;\n> > +\n> > +realloc:\n> > +\tbufp = realloc(buf, buf_len);\n> > +\n> > +\tif (bufp == NULL) {\n> > +\t\tfprintf(stderr, \"malloc error: not enough buffer\\n\");\n> > +\t\tfree(buf);\n> > +\t\treturn -ENOMEM;\n> > +\t}\n> > +\tbuf = bufp;\n> > +\tiov = msg->msg_iov;\n> > +\tiov->iov_base = buf;\n> > +\tiov->iov_len = buf_len;\n> > +\n> > +recv:\n> > +\tlen = recvmsg(fd, msg, flag);\n> > +\n> > +\tif (len < 0) {\n> > +\t\tif (errno == EINTR || errno == EAGAIN)\n> > +\t\t\tgoto recv;\n> > +\t\tfprintf(stderr, \"netlink receive error %s (%d)\\n\",\n> > +\t\t\tstrerror(errno), errno);\n> \n> free(buf);\n> \n> > +\t\treturn len;\n> \n> Maybe we should return -errno (saved before calling fprintf()) to be\n> consistent.\n> \n> > +\t}\n> > +\n> > +\tif (len == 0) {\n> > +\t\tfprintf(stderr, \"EOF on netlink\\n\");\n> \n> free(buf);\n\nWill fix these three issues.\n\n> > @@ -471,19 +516,23 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,\n> >  \n> >  \t\t\t\tif (h->nlmsg_type == NLMSG_ERROR) {\n> >  \t\t\t\t\trtnl_dump_error(rth, h);\n> > +\t\t\t\t\tfree(buf);\n> >  \t\t\t\t\treturn -1;\n> >  \t\t\t\t}\n> >  \n> >  \t\t\t\tif (!rth->dump_fp) {\n> >  \t\t\t\t\terr = a->filter(&nladdr, h, a->arg1);\n> > -\t\t\t\t\tif (err < 0)\n> > +\t\t\t\t\tif (err < 0) {\n> > +\t\t\t\t\t\tfree(buf);\n> >  \t\t\t\t\t\treturn err;\n> > +\t\t\t\t\t}\n> >  \t\t\t\t}\n> >  \n> >  skip_it:\n> >  \t\t\t\th = NLMSG_NEXT(h, msglen);\n> >  \t\t\t}\n> >  \t\t}\n> > +\t\tfree(buf);\n> \n> We only free the last buffer returned by rtnl_recvmsg() this way. IMHO\n> this free(buf) should be moved inside the loop.\n\nDo you mean the outside while loop or the for loop? I think we could not put\nit inside the for loop, because we may need the buf multi times based on arg.\n\n\twhile (1) {\n\t\tstatus = rtnl_recvmsg(rth->fd, &msg, &buf);\n\n\t\tfor (a = arg; a->filter; a++) {\n\t\t\tstruct nlmsghdr *h = (struct nlmsghdr *)buf;\n\t\t\twhile (NLMSG_OK(h, msglen)) {\n\t\t\t\t[...]\nskip_it:\n\t\t\t\th = NLMSG_NEXT(h, msglen);\n\t\t\t}\n\t\t}\n\t\tfree(buf);\n\t\t[...]\n\t}\n\nThanks\nHangbin","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"ddaK8vup\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xx76R3JJmz9rxm\n\tfor <patchwork-incoming@ozlabs.org>;\n\tTue, 19 Sep 2017 13:05:35 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751053AbdISDFc (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tMon, 18 Sep 2017 23:05:32 -0400","from mail-pg0-f66.google.com ([74.125.83.66]:37134 \"EHLO\n\tmail-pg0-f66.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750921AbdISDFb (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Mon, 18 Sep 2017 23:05:31 -0400","by mail-pg0-f66.google.com with SMTP id v5so1276840pgn.4\n\tfor <netdev@vger.kernel.org>; Mon, 18 Sep 2017 20:05:31 -0700 (PDT)","from leo.usersys.redhat.com ([209.132.188.80])\n\tby smtp.gmail.com with ESMTPSA id\n\to5sm966617pfh.67.2017.09.18.20.05.28\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tMon, 18 Sep 2017 20:05:30 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=3wAe3Jc5ZUlgguYTf7bHYyNCTvLixfAek+h/OOePhrU=;\n\tb=ddaK8vupo6P6FBjN4G5H19aRp78cRkG0yIQRBLozKXi4aoqHRuFcjcnXtYAhdI0c5s\n\tqH0N88iVu0oXCG/FbwV3+Rn2RZZtTKVjmIiyZUZOY8OGPj54N02Kb0xY7F3YfisMzL2K\n\tTiQn9RfyagvbD4DCrr2BKF2gG3YErYKluXMtGqREdk8WOmsB3tN9xBv8rnAKJPajYeQ8\n\t9m3eAzqtRhrZOlyR2t8x505UdYlk4ND0kJXeRIrP0kpAnS4pKYz2jhk8RmTVXLJ/Wz/u\n\tfGj0D8fr6CrySXpNNl0SZB3YGg3ZleITTyUe3kinNicGy6cOwraATJMY8E7pdY8zz0Lq\n\twQtg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=3wAe3Jc5ZUlgguYTf7bHYyNCTvLixfAek+h/OOePhrU=;\n\tb=QPUW6jFC7SJmY0E7T9xNGGomv86dn8o7kQo6qIcmwy9QK24dUKXT3BWcMsX8a2o2Ho\n\tZNyghrBNd6hG40/QxNreF9w7WC6zCG27N2B6jRHWqLl8/zFNpJVPtPqNLLoCcKyuHvYD\n\teybtQqAxhWYZUrybWRKzz3imBKjwIi68dKwxS1wHKBmv/hr4srVA7t7nbkaZpn+gx3RC\n\tv2QT61LGrem2NncN7oJTrpN+cjQTVuz7w49CStiHCW/uMwv5MyXVHZkBurEbTZGqMZ/2\n\tBReEv8nNrNkaVBB0tSKaKMdYz1NEhXxRWb/e1lBIqpuvfAtMlTWkq2W+C8eIn7ehaOrL\n\tWwow==","X-Gm-Message-State":"AHPjjUiq8SWVIunH4rJzDLxdBmdkSd/W1LEPecsKiqb378OjqyKLb+u4\n\t6LLdN6Il0/SN96oD","X-Google-Smtp-Source":"AOwi7QBr+2Ym0bUSkJKj1bhS5iO0DvK2k6ovti+eFVCGgiM7WUZ2GUNKnzmXlUqQBs3l0qQXGf/lIw==","X-Received":"by 10.98.220.209 with SMTP id c78mr615238pfl.37.1505790331007;\n\tMon, 18 Sep 2017 20:05:31 -0700 (PDT)","Date":"Tue, 19 Sep 2017 11:05:20 +0800","From":"Hangbin Liu <liuhangbin@gmail.com>","To":"Michal Kubecek <mkubecek@suse.cz>","Cc":"netdev@vger.kernel.org, Stephen Hemminger <stephen@networkplumber.org>,\n\tPhil Sutter <phil@nwl.cc>","Subject":"Re: [PATCHv2 iproute2 1/2] lib/libnetlink: re malloc buff if size is\n\tnot enough","Message-ID":"<20170919030520.GL5465@leo.usersys.redhat.com>","References":"<1505296780-8444-1-git-send-email-liuhangbin@gmail.com>\n\t<1505296780-8444-2-git-send-email-liuhangbin@gmail.com>\n\t<20170918075504.gdx4q7je5uneefgg@unicorn.suse.cz>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170918075504.gdx4q7je5uneefgg@unicorn.suse.cz>","User-Agent":"Mutt/1.7.0 (2016-08-17)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1770898,"web_url":"http://patchwork.ozlabs.org/comment/1770898/","msgid":"<20170919114808.pisr2lqenhtyisyj@unicorn.suse.cz>","list_archive_url":null,"date":"2017-09-19T11:48:08","subject":"Re: [PATCHv2 iproute2 1/2] lib/libnetlink: re malloc buff if size is\n\tnot enough","submitter":{"id":11892,"url":"http://patchwork.ozlabs.org/api/people/11892/","name":"Michal Kubecek","email":"mkubecek@suse.cz"},"content":"On Tue, Sep 19, 2017 at 11:05:20AM +0800, Hangbin Liu wrote:\n> On Mon, Sep 18, 2017 at 09:55:05AM +0200, Michal Kubecek wrote:\n> > > @@ -471,19 +516,23 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,\n> > >  \n> > >  \t\t\t\tif (h->nlmsg_type == NLMSG_ERROR) {\n> > >  \t\t\t\t\trtnl_dump_error(rth, h);\n> > > +\t\t\t\t\tfree(buf);\n> > >  \t\t\t\t\treturn -1;\n> > >  \t\t\t\t}\n> > >  \n> > >  \t\t\t\tif (!rth->dump_fp) {\n> > >  \t\t\t\t\terr = a->filter(&nladdr, h, a->arg1);\n> > > -\t\t\t\t\tif (err < 0)\n> > > +\t\t\t\t\tif (err < 0) {\n> > > +\t\t\t\t\t\tfree(buf);\n> > >  \t\t\t\t\t\treturn err;\n> > > +\t\t\t\t\t}\n> > >  \t\t\t\t}\n> > >  \n> > >  skip_it:\n> > >  \t\t\t\th = NLMSG_NEXT(h, msglen);\n> > >  \t\t\t}\n> > >  \t\t}\n> > > +\t\tfree(buf);\n> > \n> > We only free the last buffer returned by rtnl_recvmsg() this way.\n> > IMHO this free(buf) should be moved inside the loop.\n> \n> Do you mean the outside while loop or the for loop? I think we could\n> not put it inside the for loop, because we may need the buf multi\n> times based on arg.\n\nSorry for the confusion, you are right, this part is correct. I misread\nthe indentation.\n\nMichal Kubecek","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xxLjX72btz9s7m\n\tfor <patchwork-incoming@ozlabs.org>;\n\tTue, 19 Sep 2017 21:48:16 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751772AbdISLsP (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tTue, 19 Sep 2017 07:48:15 -0400","from mx2.suse.de ([195.135.220.15]:32854 \"EHLO mx1.suse.de\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1751753AbdISLsO (ORCPT <rfc822;netdev@vger.kernel.org>);\n\tTue, 19 Sep 2017 07:48:14 -0400","from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx1.suse.de (Postfix) with ESMTP id F38415CBBF;\n\tTue, 19 Sep 2017 11:48:12 +0000 (UTC)","by unicorn.suse.cz (Postfix, from userid 1000)\n\tid 81587A0F06; Tue, 19 Sep 2017 13:48:08 +0200 (CEST)"],"X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","Date":"Tue, 19 Sep 2017 13:48:08 +0200","From":"Michal Kubecek <mkubecek@suse.cz>","To":"Hangbin Liu <liuhangbin@gmail.com>","Cc":"netdev@vger.kernel.org, Stephen Hemminger <stephen@networkplumber.org>,\n\tPhil Sutter <phil@nwl.cc>","Subject":"Re: [PATCHv2 iproute2 1/2] lib/libnetlink: re malloc buff if size is\n\tnot enough","Message-ID":"<20170919114808.pisr2lqenhtyisyj@unicorn.suse.cz>","References":"<1505296780-8444-1-git-send-email-liuhangbin@gmail.com>\n\t<1505296780-8444-2-git-send-email-liuhangbin@gmail.com>\n\t<20170918075504.gdx4q7je5uneefgg@unicorn.suse.cz>\n\t<20170919030520.GL5465@leo.usersys.redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170919030520.GL5465@leo.usersys.redhat.com>","User-Agent":"NeoMutt/20170421 (1.8.2)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}}]