This is the mail archive of the ecos-discuss@sourceware.org mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: In-built shell for Redboot


> I sent this out originally and never heard back from anyone -- I'm game
> for feedback.  It uses a command definition structure similar to
> Redboot.

Hi Chris 

I took a look at this code when you first posted it. If i remember
correctly there were a few things i didn't like, or at least would
require some work. 

1) It is not an eCos package.  Well in fact i think it should be two
   packages. The shell frame work and your standard commands.

2) I don't like the idea of the thread wrapper. Is this actually
   necessary for the shell? I would remove it if not.

3) You are only allowed one shell. It also seems not so easy to change
   the implementation to support multiple shells. I think this is
   important. I expect people might want to run one on a serial port
   and another on a network port. So you need some way to tell
   SHELL_PRINT which shell to print to etc.

4) I don't particularly like the SHELL_PRINT macros. I would prefer
   real functions, with cyg_ prefixes. They also seem to be in a funny
   place, shell_err.h?

5) You need to abstract out the opening/writing/reading/closing the
   communication pipe from the rest of the shell. It then becomes
   easier to implement a shell on a telnet port, a shell on an SSH
   port, a shell on two tins cans and a piece of string.
              
I know these were not goals of your design, so you did not think about
them. However i think it is necessary to at least address these issues
in a flexible shell framework for any code which gets committed to
anoncvs.

        Andrew

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]