[PATCH 1/1] staging: fwserial: fix resource leak

[Vladimirs Ambrosovs]

On Mon, May 25, 2015 at 12:27:22PM +0300, Dan Carpenter wrote:
> On Mon, May 25, 2015 at 02:14:32AM +0300, Vladimirs Ambrosovs wrote:
> > From: Vladimirs Ambrosovs <rodriguez.twister at gmail.com>
> > 
> 
> No need for this, we get it from your email address.
> 
> > This patch fixes the leak, which was present in fwserial driver in the
> > init function. in case the tty driver allocation failed the function
> > returned error, leaving debugfs entry in the filesystem.
> > 
> > To fix the issue additional error label was added, so that the code will
> > jump to it in case of allocation failure, and free debugfs entries.
> > 
> > Also, the additional check for debugfs_create_dir() return value was 
> > added to warn the user, that the directory was not created. Further
> > driver code checks, whether the value is NULL or not, so it is safe
> > to continue init.
> > 
> > Signed-off-by: Vladimirs Ambrosovs <rodriguez.twister at gmail.com>
> > ---
> >  drivers/staging/fwserial/fwserial.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
> > index fdb2418..27a1d77 100644
> > --- a/drivers/staging/fwserial/fwserial.c
> > +++ b/drivers/staging/fwserial/fwserial.c
> > @@ -2812,16 +2812,25 @@ static int __init fwserial_init(void)
> >  	/* XXX: placeholder for a "firewire" debugfs node */
> >  	fwserial_debugfs = debugfs_create_dir(KBUILD_MODNAME, NULL);
> >  
> > +	/* Don't need to return error if debugfs create dir failed, since
> > +	 * it is safe to continue without debugfs entry. It is being
> > +	 * checked further in the code, before usage, but we still want
> > +	 * to inform the user
> 
> If the user didn't enable debugfs then they don't want it; no need to
> notify them.  I don't think we want this warning here at all actually.
> 
> 
Thanks, that makes sense. Another approach could be just comparing to NULL:
If debugfs is disabled, then it would fail silently returning
-ENODEV, but if it is enabled and failed - the warning would be printed.
But agree, that there's not much use of it anyway, so probably it's best
to just get rid of it. Will send an updated patch shortly.

> > +	 */
> > +	if (unlikely(IS_ERR_OR_NULL(fwserial_debugfs)))
> > +		pr_warn("failed to create debugfs entry\n");
> > +
> 
> regards,
> dan carpenter
> 

BR,
Vladimirs