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