[PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()
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; } -- 1.7.10.4
Re: [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()
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.
Re: [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()
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. If desired I will drop this one. > signature.asc Description: OpenPGP digital signature
Re: [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()
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.
Re: [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()
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?
Re: [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()
On Tue, Feb 20, 2018 at 12:33 PM, Stephen Hemminger wrote: > 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? on -force, it needs to continue.
Re: [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()
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. > Okay, now is clear. Will prepare v2 according to these rules. signature.asc Description: OpenPGP digital signature
Re: [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()
On 2/20/18 1:44 PM, Roopa Prabhu wrote: > On Tue, Feb 20, 2018 at 12:33 PM, Stephen Hemminger > wrote: >> 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? > > on -force, it needs to continue. > It does. $ ip/ip -batch /tmp/ip.batch -force 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 RTNETLINK answers: Operation not permitted Command failed /tmp/ip.batch:3 Which is an improvement over today where it just exits - ignoring the force.
Re: [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()
On Tue, Feb 20, 2018 at 12:49 PM, David Ahern wrote: > On 2/20/18 1:44 PM, Roopa Prabhu wrote: >> On Tue, Feb 20, 2018 at 12:33 PM, Stephen Hemminger >> wrote: >>> 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? >> >> on -force, it needs to continue. >> > > > It does. > > $ ip/ip -batch /tmp/ip.batch -force > 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 > RTNETLINK answers: Operation not permitted > Command failed /tmp/ip.batch:3 > > Which is an improvement over today where it just exits - ignoring the force. cool :). some other iproute2 commands have also received patches to continue on -force over the last few years.