On Fri, May 10, 2019 at 10:28:14AM +0100, Bruce Richardson wrote:
> On Fri, May 10, 2019 at 01:28:09AM +0200, Thomas Monjalon wrote:
> > > > From: Marcin Hajkowski <marcinx.hajkow...@intel.com>
> > > >
> > > > Function rte_power_set_env will no longer return
> > > > success on attempt to set env in initialized state.
> > > >
> > > > Signed-off-by: Marcin Hajkowski <marcinx.hajkow...@intel.com>
> > > 
> > > Acked-by: David Hunt <david.h...@intel.com>
> > 
> > Any other comment about this deprecation notice?
> > 
> Seems ok to me, though the actual text is maybe a little unclear. It
> implies that the function will always return -1 unless the variable is
> unset when the function terminates (which seems to imply a failure case).
> What I presume is meant is that we have three possibilities:
> 
> * The variable is set by the function -> return 0
> * The varaible is already set, so no action needed -> return -1 (and set
>   rte_errno to EEXIST or EALREADY??)
> * Setting the variable failed -> return -1 (and set rte_errno to ??)
> 
> Is my understanding correct? Can the deprecation notice be improved to make
> it clear that only the middle case is the one being changed, e.g. by adding
> "in this case" to the second sentence. It might also be worthwhile calling
> out what the errno value will be to identify this failure vs regular
> failures.
> 
> /Bruce
> 
> PS: For this case, is there a reason to make it an error? Would a +1 value
> not also do, so anything non-zero implies no work done, and anything >=0
> means that the value is set? Call set on something already set doesn't
> really seem like an error case to me.

Sorry, forgot to put:

For changing the ABI itself, no issues, so with the small rewording asked
for, please add my ack.

Acked-by: Bruce Richardson <bruce.richard...@intel.com>

Reply via email to