From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <olivier.matz@6wind.com>
Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67])
 by dpdk.org (Postfix) with ESMTP id 5C62D1B178
 for <dev@dpdk.org>; Tue,  9 Oct 2018 11:10:54 +0200 (CEST)
Received: from rsa59-2-82-233-193-189.fbx.proxad.net ([82.233.193.189]
 helo=droids-corp.org)
 by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256)
 (Exim 4.89) (envelope-from <olivier.matz@6wind.com>)
 id 1g9o3b-0003i4-Ow; Tue, 09 Oct 2018 11:12:01 +0200
Received: by droids-corp.org (sSMTP sendmail emulation);
 Tue, 09 Oct 2018 11:10:46 +0200
Date: Tue, 9 Oct 2018 11:10:46 +0200
From: Olivier Matz <olivier.matz@6wind.com>
To: Andrew Rybchenko <arybchenko@solarflare.com>
Cc: David Marchand <david.marchand@6wind.com>, dev@dpdk.org,
 "Lu, Wenzhuo" <wenzhuo.lu@intel.com>,
 "Wu, Jingjing" <jingjing.wu@intel.com>,
 "Iremonger, Bernard" <bernard.iremonger@intel.com>
Message-ID: <20181009091046.hxlwveehcg5y562u@platinum>
References: <20180910054547.18494-1-david.marchand@6wind.com>
 <20180910054547.18494-3-david.marchand@6wind.com>
 <86b2ae63-f193-03ce-457d-387b785fb884@solarflare.com>
 <CALwxeUtehsZ9JZy3r0syaD9U4aF5TDKwuoK2kcgL8A9KTLcZyA@mail.gmail.com>
 <8d525334-f770-7627-4023-c61aa92f4655@solarflare.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <8d525334-f770-7627-4023-c61aa92f4655@solarflare.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Subject: Re: [dpdk-dev] [PATCH 2/3] mbuf: add a non fatal sanity check helper
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 09 Oct 2018 09:10:54 -0000

On Mon, Sep 10, 2018 at 11:33:44AM +0300, Andrew Rybchenko wrote:
> On 09/10/2018 11:24 AM, David Marchand wrote:
> > On Mon, Sep 10, 2018 at 10:12 AM, Andrew Rybchenko
> > <arybchenko@solarflare.com> wrote:
> > > +/**
> > > + * Sanity checks on a mbuf.
> > > + *
> > > + * Almost like rte_mbuf_sanity_check(), but this function gives the reason
> > > + * if corruption is detected rather than panic.
> > > + *
> > > + * @param m
> > > + *   The mbuf to be checked.
> > > + * @param is_header
> > > + *   True if the mbuf is a packet header, false if it is a sub-segment
> > > + *   of a packet (in this case, some fields like nb_segs are not checked)
> > > + * @param reason
> > > + *   A reference to a string pointer where to store the reason why a mbuf
> > > is
> > > + *   considered invalid.
> > > + * @return
> > > + *   - 0 if no issue has been found, reason is left untouched.
> > > + *   - -1 if a problem is detected, reason then points to a string
> > > describing
> > > + *     the reason why the mbuf is deemed invalid.
> > > + */
> > > +__rte_experimental
> > > +int rte_mbuf_check(const struct rte_mbuf *m, int is_header,
> > > +   const char **reason);
> > > +
> > > 
> > > 
> > > May be it would be better to return reason as return value and if it is NULL
> > > everything is OK?
> > This was what I had done with my first attempt.
> > But given the name "rte_mbuf_check", having a int (used as a bool)
> > returned makes sense to me.
> 
> Yes, good point.
> 
> > So no strong opinion here.
> 
> Me too.

I think an int is fine.

Acked-by: Olivier Matz <olivier.matz@6wind.com>