Hi,

I missed to free the buffer in cd.patch. I have attached the updated one.
Sorry for the inconvenience caused.

Cheers,
Ketan

On Tue, Apr 3, 2012 at 11:41 AM, Vojtech Horky <[email protected]>wrote:

> Hello Ketan,
> thanks for your patch and your interest in HelenOS.
>
> Dne 2. dubna 2012 21:24 Ketan Singh <[email protected]> napsal(a):
> > Hello everyone,
> >
> > I am a GSOC 2012 aspirant and would like to contribute to HelenOS
> project. I
> > would like to submit  a very simple patch to allow jump to previous
> working
> > directory in HelenOS using 'cd -' command. Please have a look at it. Any
> There are few issues with your patch. Sorry, it is a bit longer but
> today I have somehow problems with compressing the text ;-).
>
> First, leave the whole path to the file in the patch. That is because
> filenames across directories are not unique and vfs.c is in libc as
> well as in VFS server. Although it is pretty clear which vfs.c you
> meant, it just makes things easier. Thanks.
>
> Next, the way you test for '-' is not correct because you just test
> the first character. That means that 'cd -whatever' has the same
> meaning as 'cd -something-else' and 'cd -' which is something you
> probably have not intended.
>
> When you start HelenOS and the first command you type is 'cd -', you
> will get an error proclaiming that "Destination path is too long".
> Such message is very misleading. I suspect that the cause is the
> second hunk where you test pwd_path for NULL. Can you, please, clarify
> what this code shall do? I probably missed something...
>
> And the last thing is that you put this functionality inside libc.
> Although it might seem to be a nice feature when the library itself
> remembers previous directory, it is not. First of all, it complicates
> working with directory named '-'. Although such name is weird, it is
> perfectly valid and your patch makes it - from some point of view -
> inaccessible. Next, it makes the function more powerful than it should
> be. That makes its description (documentation) more complicated and in
> my humble opinion, functions in base system library shall have as
> simple (unambiguous) functionality as possible. I think that the
> correct approach is to implement this functionality inside shell
> itself. But even there, an option to override the meaning - to allow
> cding to '-' - has to be present.
>
> Thanks again. I would be glad to apply your patch once you address, or
> explain, the issues mentioned above.
>
> Cheers,
> - Vojta
>
>
> > suggestions are most welcome. I am a student at Indian Institute of
> > Technology Guwahati in my senior year pursuing Bachelors in Computer
> > Science. I will contribute more patches as I delve deeper in the code. I
> am
> > interested in the following projects order in decreasing order of
> > preference:
> >
> > 1. Support PAE on ia32
> > 2. Improve the hash table implementation.
> > 3. Graceful system shutdown.
> >
> > I have also contributed to PintOS (
> > http://www.scs.stanford.edu/10wi-cs140/pintos/pintos.html ) as an
> academic
> > project in my university. I think I will do well as a part of HelenOS
> team
> > and will help in improving the system.
> >
> > Cheers,
> > Ketan Singh
> > Senior Undergraduate Student,
> > Department of Computer Science, Indian Institute of Technology Guwahati
> > India
> >
> > _______________________________________________
> > HelenOS-devel mailing list
> > [email protected]
> > http://lists.modry.cz/cgi-bin/listinfo/helenos-devel
> >
>
> _______________________________________________
> HelenOS-devel mailing list
> [email protected]
> http://lists.modry.cz/cgi-bin/listinfo/helenos-devel
>

Attachment: cd.patch
Description: Binary data

_______________________________________________
HelenOS-devel mailing list
[email protected]
http://lists.modry.cz/cgi-bin/listinfo/helenos-devel

Reply via email to