Hi, On Sun, Mar 25, 2012 at 10:26 PM, Jakub Jermar <[email protected]> wrote: > > There is one thing about your branch though. You named it mainline, > which suggest you will only use it to track mainline changes. For > branches that aim to provide a fix or a new feature, we would normally > pick some descriptive name of that feature or a bugfix, such as 'ls' or > 'bdsh' in your case. I know Launchpad is not very intuitive in this > regard as it allows several approaches (branches vs. projects), but I > usually have branches in the following form: > lp:~jakub/helenos/<branch_name>. It is even possible to create a branch > like that on Launchpad from command line once you have your SSH keys > uploaded to it.
The branch is now in the form lp:~vivek-cs-iitr/helenos/ls. I am getting to understand bazaar better now. > The patch looks visually good now. I also tried running your changes and > found several small issues. > > There seems to be a file descriptor leak in ls_recursive() after it > calls a nested ls_recursive(): > > ret = ls_recursive(subdir_path, subdirp); > + closedir(subdirp); > > Without the closedir(), you will not be able to successfully run ls -R / > more than once or twice because the whole bdsh task will run out of > descriptors. > > I also suspect ls_recursive() leaks the entire dir_list list and also > for each dir_elem_t on it, it leaks the name member, can you confirm? > There are indeed memory leaks. How foolish of me, that i didn't test the code properly! I found that ls_recursive() also leaks subdir_path in addition to those you mentioned. Sorry, I will take care of these things in future. Thanks for taking the trouble to point them. The fixed changes are now reflected at http://bazaar.launchpad.net/~vivek-cs-iitr/helenos/ls/revision/1441 . The patch can be downloaded from http://bazaar.launchpad.net/~vivek-cs-iitr/helenos/ls/diff/1441 . Thanks, Vivek Prakash
_______________________________________________ HelenOS-devel mailing list [email protected] http://lists.modry.cz/cgi-bin/listinfo/helenos-devel
