From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl0-f66.google.com ([209.85.160.66]:36916 "EHLO mail-pl0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751343AbeBZVgF (ORCPT ); Mon, 26 Feb 2018 16:36:05 -0500 Received: by mail-pl0-f66.google.com with SMTP id ay8so10043324plb.4 for ; Mon, 26 Feb 2018 13:36:04 -0800 (PST) Date: Mon, 26 Feb 2018 13:35:56 -0800 From: Stephen Hemminger To: Serhey Popovych Cc: David Ahern , netdev@vger.kernel.org Subject: Re: [PATCH iproute2-next v3 8/8] iplink: Reduce number of arguments to iplink_parse() Message-ID: <20180226133556.1e24b6d1@xeon-e3> In-Reply-To: References: <1519304526-18848-1-git-send-email-serhe.popovych@gmail.com> <1519304526-18848-9-git-send-email-serhe.popovych@gmail.com> <20180226100638.789a770d@xeon-e3> <4df74e2b-7248-1b48-3b2b-0ca2b10ea7de@gmail.com> <0834a77a-4b9c-d132-990b-f0afb92768fb@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; boundary="Sig_/cSV7Js6LKBJfMtCMrPDgxy+"; protocol="application/pgp-signature" Sender: netdev-owner@vger.kernel.org List-ID: --Sig_/cSV7Js6LKBJfMtCMrPDgxy+ Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Mon, 26 Feb 2018 22:09:03 +0200 Serhey Popovych wrote: > Serhey Popovych wrote: > > David Ahern wrote: =20 > >> On 2/26/18 11:20 AM, Serhey Popovych wrote: =20 > >>> Stephen Hemminger wrote: =20 > >>>> On Thu, 22 Feb 2018 15:02:06 +0200 > >>>> Serhey Popovych wrote: > >>>> =20 > >>>>> +struct iplink_parse_args { > >>>>> + const char *dev; > >>>>> + const char *name; > >>>>> + const char *type; > >>>>> + > >>>>> + /* This definitely must be the last one and initialized > >>>>> + * by the caller of iplink_parse() that will initialize rest. > >>>>> + */ > >>>>> + struct iplink_req *req; > >>>>> +}; > >>>>> + =20 > >>>> > >>>> No control block please. =20 > >>> Accepted. > >>> =20 > >>>> If you have too many arguments, then that means you need to do > >>>> some refactoring. =20 > >>> > >>> So using structure as single argument to a function isn't an option? > >>> =20 > >>>> =20 > >>> > >>> =20 > >> > >> As I mentioned before, iplink_parse should not be used by vxcan or veth > >> as they only want a subset of the parsing. Once you take those users > >> out, iplink_parse becomes local to iplink.c with a single user. In whi= ch > >> case I suspect the compiler will always inline the function so no > >> refactoring on the number of arguments is needed. > >> =20 > > I will implement cut down function to parse vxcan and veth peer device > > parameters and reuse it in iplink_parse() to avoid code duplications. > >=20 > > But my final goal not to refactor on number of arguments to parse, > > that's side product of this series, I want to take @name, @dev and > > other parameters for later use. In ->parse_opt() modules @name, @dev > > and others are not available easily. It seems only way to get them is > > to parse supplied netlink buffer. > >=20 > > =20 > While looking on how to make iplink_parse_light() that will be used with > vxcan and veth I found following problems: >=20 > 1) need to copy nearly all parameters parsing code (except vf, alias, > carrier, master, protodown, link-netnsid, addrgenmode). this will > be mitigated by re using iplink_parse_light() in iplink_parse() >=20 > 2) how to add attributes like IFLA_GROUP? in caller? this will give > even more code duplications. >=20 > 3) there is high risk of adding regression either via conflicting > matches() parameter in userspace or missing kernel attribute > previously supported. >=20 > Sorry, I do not like this approach. I do not want to broke anything, > I just want @name and @dev parameters in ->parse_opt(). >=20 >=20 > 2) > > =20 If the work to use common code is bigger than the code itself then why bother. There is no benefit. --Sig_/cSV7Js6LKBJfMtCMrPDgxy+ Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEn2/DRbBb5+dmuDyPgKd/YJXN5H4FAlqUfbwACgkQgKd/YJXN 5H52sRAAltSd1Ysmc6SharVnWCsq/YLn3ETjrwDpWfWWu1eQOQxEPqc6Rn52OH4t 4TU+Tjr4vTmdCCQH5osKQH/Gzo2JQyMEe88An4bVLKkHT8neDa8agYtFgq+SueDG jyVLZF4W+dH817JMLt78ukmTuSPD/AsqMEJdVns2pcDAZ3wv9YSHeaQlEiYNMQ3N W1t14BHAJFOMu7deQIYWusRMG/DTpb6Skk9khSLpn1JnW5cyEo/aODIXH6CgudXZ H0cZ1Z7i8FJRq4rmpjgfeV8cQIGcgySFtZtLcHxvaUaQOtbOhnSGkOkh87GIDnfO FJAYlVLcuL2lAgTxURxs0xzthiuqDooGG3M7GmrJDYo+y/MUfQM2j6zwfdCzgx8g BCWR86Nq0KuvSu6NeJ3jjub3QIrDtZulbL2c4aGCrKtkVH1GwUWlWTpg610wcr8X TjNG0zsRpEMGNWaZQpY1sk2hCuZs4sNvkrDC7Ugk8ceUS2k/LJZ3eu1QQIpdnSfb AX6svkJIWF7VojZkju6PMaj+WBR2B8R71OTXtSRPdSjyfDO0tXrOLvwebFAihYDk 9YAhJXLTcewnXsXSb70btfmfJijCK0Hyb/oNqORLMUQnKvD12Z5fPc5tB0PWuEQX gxPbEuyIJuENKXWA4d1KO6ZGXMqKJP0d3NXU+6uZ5ZM2layibA8= =rb67 -----END PGP SIGNATURE----- --Sig_/cSV7Js6LKBJfMtCMrPDgxy+--