On Wed, Feb 13, 2019 at 04:04:29PM +0100, Johannes Berg wrote:
> On Wed, 2019-02-13 at 22:39 +0800, Herbert Xu wrote:
> > +   if (ret != -EEXIST)
> >             return ERR_PTR(ret);
> 
> Surely that should still be "if (ret && ret != -EEXIST)" otherwise you
> return for the success case too? or "else if"?
> 
> I'd probably say we should combine all those ifs into something like this?
> 
> 
> if (ret == 0) {
>       sdata->u.mesh.mesh_paths_generation++;
>       return new_mpath;
> } else if (ret == -EEXIST) {
>       kfree(new_mpath);
>       return mpath;
> } else {
>       kfree(new_mpath);
>       return NULL;
> }
> 
> 
> Yes, that's a small change in behaviour as to when the generation
> counter is updated, but I'm pretty sure it really only needs updating
> when we inserted something, not if we found an old mpath.

You are right.  Let me fix this and repost.

Thanks!
-- 
Email: Herbert Xu <herb...@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Reply via email to