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 <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
