On Wed, Oct 24, 2018 at 02:39:47PM -0700, Andrew Morton wrote: > On Tue, 23 Oct 2018 23:48:46 -0300 Ernesto A. Fernández <ernesto.mnd.fernandez@xxxxxxxxx> wrote: > > > > Sorry, I don't follow where the new_node is put twice. Could you explain > > > in more details? Currently, it looks unclear. > > > > There is a 'goto again', as I said in the commit message. Follow the code > > and you'll see that hfs_bnode_put() is called again on the same node. > > > > > I like to assign the NULL value to the pointer. > > > > This isn't a matter of taste. > > > > > But are you sure that it's proper place? > > > > Yes, but it's always better if somebody reviews the code... > > > > > Maybe it > > > makes sense to place this assignment in the beginning of the function? > > > > Without knowing what you mean by "beginning of the function", I can't > > tell if your idea would work or not. > > I think it would be clearer to do it this way: > > --- a/fs/hfs/brec.c~a > +++ a/fs/hfs/brec.c > @@ -359,11 +359,11 @@ static int hfs_brec_update_parent(struct > > tree = fd->tree; > node = fd->bnode; > - new_node = NULL; > if (!node->parent) > return 0; > > again: > + new_node = NULL; > parent = hfs_bnode_find(tree, node->parent); > if (IS_ERR(parent)) > return PTR_ERR(parent); > > But it doesn't matter much... Right, that looks better. I don't remember why I did it this way. If you want me to resend I'd rather run some tests again, just in case.
