On Friday 31 May 2013 10:16 PM, Julia Lawall wrote: > 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 Thank you Dan & Julia. This will surely be very helpful. Harsh _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
