Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   <Daniel.Machon@microchip.com>
To:     <joe@perches.com>
CC:     <petrm@nvidia.com>, <netdev@vger.kernel.org>,
        <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
        <pabeni@redhat.com>, <vladimir.oltean@nxp.com>,
        <linux-kernel@vger.kernel.org>, <UNGLinuxDriver@microchip.com>,
        <lkp@intel.com>
Subject: Re: [PATCH net-next] net: dcb: move getapptrust to separate function
Thread-Topic: [PATCH net-next] net: dcb: move getapptrust to separate function
Thread-Index: AQHY9OgDJPilDH1/CUmOWEpqQ4wOy644Wb6AgADxvQCAAGJMgIAEdl8A
Date:   Mon, 14 Nov 2022 08:45:42 +0000
Message-ID: <Y3ICvKvpiG+Rn+2h@DEN-LT-70577>
References: <20221110094623.3395670-1-daniel.machon@microchip.com>
 <87eduaoj8g.fsf@nvidia.com> <Y23x/PSlybLqaQIS@DEN-LT-70577>
 <180a55b046e4659609cdfeea4fd979edab17f0c9.camel@perches.com>
In-Reply-To: <180a55b046e4659609cdfeea4fd979edab17f0c9.camel@perches.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?6HFSQyqy7THsw1BZLUxxSC1hq3M79AL/JkdpWNqaFU2VlC75AbSIhnKTkWVH?=
 =?us-ascii?Q?RNyY6k9HwO3w9hUa0Jh+vXiMr61QilmZeT0XuMeRVLwbtRxeQzlANHimNgG0?=
 =?us-ascii?Q?mW9DHWSof8YiuPPDvdWa5G7ji9bKbqyUOUCQPdOaZkSSe9xIf20uxxxElcSO?=
 =?us-ascii?Q?WrV6a6rZLWIzWhS4cuWjBKIuP/8ayTbJwkl1czr+dvEXgldCyRdPZPsXDxvo?=
 =?us-ascii?Q?b3116B06M0d47XWk5lFyBe0T8SMsn3ckoAgH+VqWZmmMbgBIiH/WjiemDrs6?=
 =?us-ascii?Q?fSTvO0LJuE2Vde7/dgAd6W5hg+PP3jswQtro9mqO+OYi5//32BSYY0PbTiUB?=
 =?us-ascii?Q?DsR+CHDVxs/z9VNgrEg+NFjmvlR+BhQC6QGcDeQKP/V2nNUHoqIlP88Uj290?=
 =?us-ascii?Q?2z8ipbTba/Nbz7t6gKwcRl796jY9JfHk8dILRBqXxZ9qmboEgxnoUUsyxQYe?=
 =?us-ascii?Q?jqUtxlG92AnsLJA8Gjutl7HFcit6KTE4AJ/Qp5koU4vYclka1163v3kYBSVf?=
 =?us-ascii?Q?Vp/nqTU3fefFAomitL8Pl7h743a22v9DhTelKs/YZGQuTebDAxGkzdq8MRgX?=
 =?us-ascii?Q?//aQWvQRI0mPLnwe7vTHT5dFWOXmrs5JQtJ2IdiijUl9WPRuBuO+Y/NFyyV7?=
 =?us-ascii?Q?pwAYdjsvjwqzZ4FIUeMLf7X2vI30CKlvf/0Phgi65ZTxOuqwa2FMsJSdG764?=
 =?us-ascii?Q?e3Wuiyxc1yoH/3Ro4t0028kn+B5PplLapJ2WSFAFMrzRmMCND8Cl1Chxxrhz?=
 =?us-ascii?Q?eVdRmSE3YCs0YZjSSzIOsOGXAQb65cvJIiYhmCAp4Wtf6j9sASQXbnOOb1ss?=
 =?us-ascii?Q?k1om68msHO1D7DIqVWDF6ozGYH6TAeFnvJJZFcSPRphj4keC25JJu0DDl+9N?=
 =?us-ascii?Q?5tBj/GVNQUAKgod4ej9CyqthqL8PlfxglA/m2Sozm8MvvQ1u1/pppB3lJmap?=
 =?us-ascii?Q?ZkD1fJvSm+ypsErALtvw3rPxfmOFoWWHD7f82rLcKmTbukvHpV2tyVsOxIH3?=
 =?us-ascii?Q?xkbV1PLORbZkS05c69FWteiheA4MhH3Zs5Gx21/JYNtW3wdpDNd1BGHTVXZV?=
 =?us-ascii?Q?usJ9yS+fLbTA607YlYnAIFNMIYN02uuc+6FgvtK1boUqFdxXsqM2u0MQXYzO?=
 =?us-ascii?Q?Z0eTptWxs6W+WKAwnEQ0wc+fwn9oIq2oyL5AMnnaCzzFBEf7OfaSqpTi6EvB?=
 =?us-ascii?Q?I/yBzdMxgH3Wdrzl3i2o4usGmSH6cs0Ey82nDwoATm3UWksk21imkV2YmuHK?=
 =?us-ascii?Q?DoWuuC3kXASyMnqxR3usAe72dRC7LT6ODUigluKDMuB6NHNqejGGvAE4XCpC?=
 =?us-ascii?Q?uBIheJo0OsZNhAq2zHoFac5JJnSicOtjeS17K2jLAwAESxa3Nk3ONoBB+iYh?=
 =?us-ascii?Q?bVeBfY5ZWTQ13310S50+NfAgYyP7hHLdXJGOEMB/MYwcf7OuF9gSUOXjEjQp?=
 =?us-ascii?Q?TkpEvXbT20sCO2HEWFrnvtTBN9gr/ce1N5qOJBD1eBFDnxVfgi0nWixpUhC1?=
 =?us-ascii?Q?axtQYnv22K9NtcDn2vPbVtExHqg9uqKcuwoEYngt9rFABzg69jjZy6/5f0zx?=
 =?us-ascii?Q?IWzsCxoRulZ5KWcYSnV2X+mtfv3ujXOs/OzCgkYFU+/dXfxDvcu692mNQ4MU?=
 =?us-ascii?Q?cQuZmvtT+yqoputwFG+JEYg=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <0AA2D481330DA64694691D0662968A57@namprd11.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CO6PR11MB5569.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: cbbd28c7-e73d-49b9-8a3f-08dac61c9d35
X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Nov 2022 08:45:42.4949
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3f4057f3-b418-4d4e-ba84-d55b4e897d88
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: x3RQJlyWp2VX/eh1toiiWi+JqXWYRMnJGkBL+DXodRpBvAuHMUwne58kxzJDQJJd2mXcOPDDgSLJe9tHI5fbpLBCrh1AqFdqPFwX4F0j6PY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB5035
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> > > > +
> > > > +             err =3D -EMSGSIZE;
> > > > +
> > > > +             apptrust =3D nla_nest_start(skb, DCB_ATTR_DCB_APP_TRU=
ST_TABLE);
> > > > +             if (!apptrust)
> > > > +                     goto nla_put_failure;
> > > > +
> > > > +             for (i =3D 0; i < nselectors; i++) {
> > > > +                     enum ieee_attrs_app type =3D
> > > > +                             dcbnl_app_attr_type_get(selectors[i])=
;
> > >
> > > Doesn't checkpatch warn about this? There should be a blank line afte=
r
> > > the variable declaration block. (Even if there wasn't one there in th=
e
> > > original code either.)
> >
> > Nope, no warning. And I think it has something to do with the way the l=
ine
> > is split.
>=20
> yup.
>=20
> And style trivia:
>=20
> I suggest adding error types after specific errors,
> reversing the test and unindenting the code too.
>=20
> Something like:
>=20
>         err =3D ops->dcbnl_getapptrust(netdev, selectors, &nselectors);
>         if (err) {
>                 err =3D 0;
>                 goto out;
>         }
>=20
>         apptrust =3D nla_nest_start(skb, DCB_ATTR_DCB_APP_TRUST_TABLE);
>         if (!apptrust) {
>                 err =3D -EMSGSIZE;
>                 goto out;
>         }
>=20
>         for (i =3D 0; i < nselectors; i++) {
>                 enum ieee_attrs_app type =3D dcbnl_app_attr_type_get(sele=
ctors[i]);
>                 err =3D nla_put_u8(skb, type, selectors[i]);
>                 if (err) {
>                         nla_nest_cancel(skb, apptrust);
>                         goto out;
>                 }
>         }
>         nla_nest_end(skb, apptrust);
>=20
>         err =3D 0;
>=20
> out:
>         kfree(selectors);
>         return err;
> }
>

LGTM.

The last err =3D 0 can even be removed, as I see it.
Will submit a new version with the changes suggested.

/ Daniel