On 10/02/13 15:32, Dave Chinner wrote:
On Wed, Oct 02, 2013 at 07:51:11AM -0500, Mark Tinguely wrote:Fix the leak of kernel memory in xfs_dir2_node_removename() when xfs_dir2_leafn_remove() returns an error code. Found by Coverity in userspace same patch applies there also. Signed-off-by: Mark Tinguely<tinguely@xxxxxxx> --- v2 corrected bad return code as pointed out by Roger Willcocks. fs/xfs/xfs_dir2_node.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) Index: b/fs/xfs/xfs_dir2_node.c =================================================================== --- a/fs/xfs/xfs_dir2_node.c +++ b/fs/xfs/xfs_dir2_node.c @@ -2105,12 +2105,12 @@ xfs_dir2_node_lookup( */ int /* error */ xfs_dir2_node_removename( - xfs_da_args_t *args) /* operation arguments */ + struct xfs_da_args *args) /* operation arguments */ { - xfs_da_state_blk_t *blk; /* leaf block */ + struct xfs_da_state_blk *blk; /* leaf block */ int error; /* error return value */ int rval; /* operation return value */ - xfs_da_state_t *state; /* btree cursor */ + struct xfs_da_state *state; /* btree cursor */ trace_xfs_dir2_node_removename(args); @@ -2132,9 +2132,10 @@ xfs_dir2_node_removename( * Didn't find it, upper layer screwed up. */ if (rval != EEXIST) { - xfs_da_state_free(state); - return rval; + error = rval; + goto done;Can you make this something like "out_free"? We tend to name jump labels according to the action that needs to be taken, like "out_unlock", "error_trans_cancel", etc... Also, this code is now "funky" in how it handles rval. it will do this: /* * Look up the entry we're deleting, set up the cursor. */ error = xfs_da3_node_lookup_int(state,&rval); if (error) rval = error; /* * Didn't find it, upper layer screwed up. */ if (rval != EEXIST) { error = rval; goto out_free; } That's kind funky with the reassignment of rval if there's an error. Better would be: /* Look up the entry we're deleting, set up the cursor. */ error = xfs_da3_node_lookup_int(state,&rval); if (error) goto out_free; /* Didn't find it, upper layer screwed up. */ if (rval != EEXIST) { error = rval; goto out_free; } Cheers, Dave.
nod. thank --Mark. _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
