Hi Ketan, thanks for the patch. I inlined some comments below but the current version of the patch looks good to me and I will integrate it into mainline. Though, it make take some time. Once again, thanks.
Dne 5. dubna 2012 13:07 Ketan Singh <[email protected]> napsal(a): >> > inside the libc ( I think that should be its proper place. ). Also the >> > changes in chdir are minimal now ( It sets the pwd parameter. ). >> > Regarding the second issue ( '-' could be a valid filename ), we have to >> > compromise somewhere. This is similar to the case of 'cd ..' . I could >> > implement something like 'cd ...' to maintain the format and forbid the >> > user >> Probably I didn't express myself clearly. My point is that if we add >> such behavior, we must provide way to override it. For example, 'cd -' >> to switch to previous directory and 'cd -- -' to switch to directory >> '-' (taking '--' as way to end option specification). Using different >> syntax is not a solution. > > > I have implemented the override feature in the new patch. Please have a look > at it. If you find any problems/ suggestions please let me know. There is just one little thing I might change. Maybe the -- can be used with any directory. I believe that this could be fixed with 2 lines of code ;-). >> Last thing, the line memset(buffer, 0, sizeof(PATH_MAX)) looks odd. >> First, you probably did not want to apply sizeof() here. Plus there is >> no need to zero the buffer because getpwd is safe - the string is >> always zero terminated. Exception is when NULL is returned due to >> error and the buffer is untouched. Thus simple buffer[0] = 0 to create >> empty string shall be sufficient. > > > Actually, I borrowed the convention from cmds/modules/ls/ls.c. I have > removed that from the current patch. Aha, okay then. Sorry. It is there for debugging purposes probably. > > All the issues that you suggested are fixed in the current patch. Please > have a look at it. Thank you very much for your feedback. I am always open > to any more suggestions. Thanks again for the patch! Cheers, - Vojta _______________________________________________ HelenOS-devel mailing list [email protected] http://lists.modry.cz/cgi-bin/listinfo/helenos-devel
