Re: [Patch 2/2] Staging: winbond: Memory & urb freed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 31 May 2013, Dan Carpenter wrote:

> On Fri, May 31, 2013 at 09:48:58PM +0530, Harsh Kumar wrote:
> >
> >
> > On Friday 31 May 2013 09:27 PM, Dan Carpenter wrote:
> > > On Fri, May 31, 2013 at 09:02:11PM +0530, Harsh Kumar wrote:
> > >> Memory & urb should be freed before exiting from the function, I think.
> > >>
> > >
> > > They are freed in Wb35Reg_EP0VM_complete() so this patch will make
> > > the system crash right away.  Btw, there are tons of real bugs that
> > > I know about but which I don't fix because I don't know what the
> > > right thing to do is.
> > >
> >
> > Ohh! Sorry, I missed it. That is why I was not sure about submitting this
> > change. I need to be more thorough in checking this stuff.
> >
> > I have general question - Generally, shouldn't allocation and freeing up the
> > memory be in the same function so that it is easy to make sure that everything
> > is freed up after completion of task? Is it because there maybe certain cases
> > where that may not be feasible or desirable to do so?
>
> Unfortunately kernel programming will never be 100% easy...  :P  In
> the end you will learn the tricks just like I did.  Here was my
> thought process here:
> 1) The commit message was not convincing.
> 2) The memory was saved to reg->reg_first.
> 3) Follow the Wb35Reg_EP0VM_start() call to the urb_submit point.
> 4) See that we pass Wb35Reg_EP0VM_complete() to urb_submit function.
>    I have worked on usb drivers before so I suspected that
>    Wb35Reg_EP0VM_complete() frees the urb.
> 5) Vefiried that this is true.  Done.
>
> Or alternatively:
> 3) Find reg_first in Wb35Reg_EP0VM_complete() and see that we free
>    it.
>
> Make sure you have cscope configured in vim.  Also in vim the '*'
> button searches.
>
> If you think you have found a bug but you're not sure, feel free to
> ask on kernel janitors.  Probably for new code it's not the right
> mailing list but for code audits it's fine.

I tend to look at other examples nearby.  If other similar functions are
freeing something, then it probably needs to be freed.  If not, then it
might seem more promising to try to figure out why.

julia
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux