On Mon, 28 Sep 2020 17:44:55 +0200
Michal Kubecek <mkube...@suse.cz> wrote:
> On Thu, Sep 24, 2020 at 09:27:57PM +0200, Ivan Vecera wrote:
> > Fixes: f2c17e107900 ("netlink: add netlink handler for gfeatures (-k)")
> >
> > Cc: Michal Kubecek <mkube...@suse.cz>
> > Signed-off-by: Ivan Vecera <ivec...@redhat.com>
> > ---
> > netlink/features.c | 9 +++++----
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/netlink/features.c b/netlink/features.c
> > index 3f1240437350..b2cf57eea660 100644
> > --- a/netlink/features.c
> > +++ b/netlink/features.c
> > @@ -112,16 +112,17 @@ int dump_features(const struct nlattr *const *tb,
> > unsigned int *feature_flags = NULL;
> > struct feature_results results;
> > unsigned int i, j;
> > - int ret;
> > + int ret = 0;
> >
> > ret = prepare_feature_results(tb, &results);
> > if (ret < 0)
> > return -EFAULT;
> >
> > - ret = -ENOMEM;
> > feature_flags = calloc(results.count, sizeof(feature_flags[0]));
> > - if (!feature_flags)
> > + if (!feature_flags) {
> > + ret = -ENOMEM;
> > goto out_free;
> > + }
> >
> > /* map netdev features to legacy flags */
> > for (i = 0; i < results.count; i++) {
> > @@ -184,7 +185,7 @@ int dump_features(const struct nlattr *const *tb,
> >
> > out_free:
> > free(feature_flags);
> > - return 0;
> > + return ret;
> > }
> >
> > int features_reply_cb(const struct nlmsghdr *nlhdr, void *data)
> > --
> > 2.26.2
>
> The patch is correct but relying on ret staying zero through the whole
> function is rather fragile (it could break when adding more checks in
> the future) and it also isn't consistent with the way this is done in
> other functions.
>
> AFAICS you could omit the first hunk and just add "ret = 0" above the
> out_free label.
>
> Michal
OK, will do.
Ivan
