Re: [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()

[Roopa Prabhu]

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(, , 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.

Re: [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()

[David Ahern]

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(, , 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()

[Serhey Popovych]

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(, , 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()

[Roopa Prabhu]

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(, , 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()

[Stephen Hemminger]

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(, , 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()

[David Ahern]

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(, , 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()

[Serhey Popovych]

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(, , 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()

[Stephen Hemminger]

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(, , 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.