Hi Jakub, I have made necessary changes, please review it.
thanks, On Fri, Apr 5, 2013 at 4:09 AM, Jakub Hrozek <jhro...@redhat.com> wrote: > On Thu, Apr 04, 2013 at 02:44:47AM +0530, Abhishek Singh wrote: > > Hi Jakub, > > > > I have done requested changes and have attached the patch with this mail. > > Kindly review it. > > > > thanks, > > > > > > Hi Abhishek, > > this is much better and we're almost there. I only have a couple of > comments before I push the patch: > > 1) I couldn't apply the patch cleanly. Can you make sure that the > patches you send to the list can be applied on top of origin/master? In > most cases, all you have to do is continuously rebase using "git rebase > origin/master" and fix the conflicts. > > If there are any patches that needs to be applied along with this one, > feel free to send them along in the same e-mail as another attachment. > > 2) because I couldn't apply the patch I only did visual review. I have > one nitpick: > > +static char *get_filepath(char path[23]) > +{ > + int ret; > + > + strncpy(path, FILE_PATH, 23); > > I'm not fond of the magic constant "23" used here. I think it would be > better to either declare that path is always PATH_MAX long or pass its > size as additional parameter and declare path as "char *". > > 3) The get_dirfd() function assigns a DIR pointer but never calls > closedir() to free this resource. It's not a big deal in tests, but we > should still try to make the code as good as possible. > > I think in this case actually a "fixture" might be a good idea. A fixture > in unit tests is a function that gets run before the test is executed > and another after the test is done. I think for this test it might be a > good idea to include a setup fixture to open the directory with opendir() > and a teardown fixture to close the handle with closedir(). > > Feel free to use either a global variable or the **state pointer for > storing the dir handle, both would work for me. If you need more > information on using the cmocka fixtures, some documentation is > available at: > https://cmockery.googlecode.com/svn/trunk/doc/index.html#Test_State > > Feel free to ping me on #sssd if you need any help. > _______________________________________________ > sssd-devel mailing list > sssd-devel@lists.fedorahosted.org > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel >
0001-cmocka-unittest-for-find_uid-added.patch
Description: Binary data
0001-cmocka-unittest-for-io-added.patch
Description: Binary data
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel