From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl0-f66.google.com ([209.85.160.66]:39497 "EHLO mail-pl0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751327AbeBTUdS (ORCPT ); Tue, 20 Feb 2018 15:33:18 -0500 Received: by mail-pl0-f66.google.com with SMTP id s13so8072229plq.6 for ; Tue, 20 Feb 2018 12:33:17 -0800 (PST) Date: Tue, 20 Feb 2018 12:33:15 -0800 From: Stephen Hemminger To: David Ahern Cc: Serhey Popovych , netdev@vger.kernel.org Subject: Re: [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit() Message-ID: <20180220123315.7b94bcf9@xeon-e3> In-Reply-To: <1bc704b8-ea19-ed58-b4f2-60123692055c@gmail.com> References: <1519155598-22137-1-git-send-email-serhe.popovych@gmail.com> <1519155598-22137-2-git-send-email-serhe.popovych@gmail.com> <20180220120821.61fe7d74@xeon-e3> <1bc704b8-ea19-ed58-b4f2-60123692055c@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org List-ID: On Tue, 20 Feb 2018 13:27:21 -0700 David Ahern wrote: > On 2/20/18 1:17 PM, Serhey Popovych wrote: > > Stephen Hemminger wrote: > >> On Tue, 20 Feb 2018 21:39:51 +0200 > >> Serhey Popovych wrote: > >> > >>> Signed-off-by: Serhey Popovych > >>> --- > >>> ip/iplink.c | 12 ++++++------ > >>> 1 file changed, 6 insertions(+), 6 deletions(-) > >>> > >>> diff --git a/ip/iplink.c b/ip/iplink.c > >>> index 74c377c..a2c8108 100644 > >>> --- a/ip/iplink.c > >>> +++ b/ip/iplink.c > >>> @@ -653,7 +653,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, > >>> NEXT_ARG(); > >>> if (xdp_parse(&argc, &argv, req, dev_index, > >>> generic, drv, offload)) > >>> - exit(-1); > >>> + return -1; > >>> } else if (strcmp(*argv, "netns") == 0) { > >>> NEXT_ARG(); > >>> if (netns != -1) > >>> @@ -972,12 +972,12 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv) > >>> if (!dev) { > >>> fprintf(stderr, > >>> "Not enough information: \"dev\" argument is required.\n"); > >>> - exit(-1); > >>> + return -1; > >>> } > >>> if (cmd == RTM_NEWLINK && index) { > >>> fprintf(stderr, > >>> "index can be used only when creating devices.\n"); > >>> - exit(-1); > >>> + return -1; > >>> } > >>> > >>> req.i.ifi_index = ll_name_to_index(dev); > >>> @@ -1392,7 +1392,7 @@ static int do_set(int argc, char **argv) > >>> if (!dev) { > >>> fprintf(stderr, > >>> "Not enough of information: \"dev\" argument is required.\n"); > >>> - exit(-1); > >>> + return -1; > >>> } > >>> > >>> if (newaddr || newbrd) { > >>> @@ -1553,7 +1553,7 @@ static int iplink_afstats(int argc, char **argv) > >>> fprintf(stderr, > >>> "Command \"%s\" is unknown, try \"ip link help\".\n", > >>> *argv); > >>> - exit(-1); > >>> + return -1; > >>> } > >>> > >>> argv++; argc--; > >>> @@ -1648,5 +1648,5 @@ int do_iplink(int argc, char **argv) > >>> > >>> fprintf(stderr, "Command \"%s\" is unknown, try \"ip link help\".\n", > >>> *argv); > >>> - exit(-1); > >>> + return -1; > >>> } > >> > >> Not sure I like this. If given bad input in batch it is better to stop and exit > >> rather than continuing with more bad data. > > > > When preparing this change I think in opposite direction: we want to > > continue batch mode if single line is broken. > > > > batch mode needs to stop on the line that fails. That said, batch still > fails with the /exit/return/ change > > $ cat /tmp/ip.batch > li sh > li foo > li add veth1 type veth peer name veth2 > > Current command > $ ip -batch /tmp/ip.batch > 1: lo: mtu 65536 qdisc noqueue state UNKNOWN mode > DEFAULT group default qlen 1000 > link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 > > > Command "foo" is unknown, try "ip link help". > $ echo $? > 255 > > $ ip/ip -batch /tmp/ip.batch > 1: lo: mtu 65536 qdisc noqueue state UNKNOWN mode > DEFAULT group default qlen 1000 > link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 > > Command "foo" is unknown, try "ip link help". > Command failed /tmp/ip.batch:2 > dsa@kenny:~/iproute2/iproute2-next.git$ echo $? > 1 > > I like that better because it tells me the line that fails. Normally ip batch will exit on errors. The question is what about -force?