public inbox for ecos-discuss@sourceware.org
 help / color / mirror / Atom feed
From: "Chris Zimman" <czimman@bloomberg.com>
To: "Andrew Lunn" <andrew@lunn.ch>
Cc: <ecos-discuss@ecos.sourceware.org>
Subject: RE: [ECOS] In-built shell for Redboot
Date: Thu, 01 Feb 2007 11:52:00 -0000	[thread overview]
Message-ID: <4B24CC52E8A6EB4A95B414126492987C033480D6@lo2525.corp.bloomberg.com> (raw)

Hi Andrew

(1)  I could make it a package, but it was pulled out of a much larger
piece of code, and hadn't as of yet.  Feel free if you wish.

(2)  The thread wrappers have nothing to do with the shell itself, but
they do provide something very useful (to us at least).  They give a
sort of 'process emulation'.  When you create a thread using the
wrappers, when it exits, it automatically gets cleaned up.

(3)  Yes, it doesn't support multiple shells, but this wouldn't be
terribly hard to add.

(4)  I would rather have macros because I can conditionally compile
stuff out that I don't want without the overhead of having to go into a
function to check eg.

Cyg_xxxxxxx_printf()
{
	if(!debug_enabled) return;

	...
}

Rather, in our code, we could say:

SHELL_DEBUG_PRINT(....)

etc.

And just by either having -DDEBUG or not, we could turn on all kinds of
debugging output and have it stripped for release.

GCC supports variadic macros just fine as far as I can tell, so I don't
see a huge downside.

(5)  I agree, that would be useful

In general, I didn't have the idea of adding a fully fledged shell to
eCos, but rather, we needed something simple that we could interact with
the target platform under.  That's what this provided.

I'm just providing this in case anyone wants to use it, it served our
purposes fine.  If someone wants to write a better and/or more full
fledged shell, by all means do so, but in the mean time, this may prove
useful to some people.

--Chris

-----Original Message-----
From: Andrew Lunn [mailto:andrew@lunn.ch] 
Sent: 01 February 2007 11:41
To: Chris Zimman
Cc: ecos-discuss@ecos.sourceware.org
Subject: Re: [ECOS] 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

             reply	other threads:[~2007-02-01 11:52 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-01 11:52 Chris Zimman [this message]
  -- strict thread matches above, loose matches on Subject: below --
2007-02-01 10:08 Chris Zimman
2007-02-01 11:41 ` Andrew Lunn
2007-01-30 21:13 Harish Talanki
2007-01-30 21:46 ` Andrew Lunn
2007-02-01  1:56   ` wang cui
2007-02-01  7:48     ` Andrew Lunn
2007-02-01 17:23     ` Harish Talanki
2007-02-01 18:51     ` Sergei Gavrikov
2007-02-02  2:46       ` wang cui
2007-02-02 12:41         ` Gary Thomas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4B24CC52E8A6EB4A95B414126492987C033480D6@lo2525.corp.bloomberg.com \
    --to=czimman@bloomberg.com \
    --cc=andrew@lunn.ch \
    --cc=ecos-discuss@ecos.sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).