RE: Pipe Library

["Cameron Taylor" <ctaylor at waverider dot com>]

I've made most of the changes suggested below by Andrew Lunn:

1. Corrected copyright statements.
2. Dropped references to "Library" everywhere, including changing from
"pipelib_" to "pipe_"
3. Replaced pipelib_purge() with a note that "fsync()" will discard all
contents.
4. Replaced calls to kapi functions with drv_api functions.
	Question: Are there driver versions of cyg_selrecord() and cyg_selwakeup()?
5. Removed the timeouts. Equivalent functionality, with more flexibility, is
available with select(). As a telling aside, while we originally used the
timeouts in a few places, these have since been replaced - for various
reasons - with calls to select().

The ecos.db entry becomes:
package CYGPKG_IO_PIPELIB {
    alias        { "Pipe Driver"}
    directory    io/pipe
    script       pipe.cdl
    description  "This package contains a device driver for pipes."
}


I'll leave the block on open for someone else.

Cameron

-----Original Message-----
From: Andrew Lunn [mailto:andrew@lunn.ch]
Sent: Tuesday, December 28, 2004 12:53 PM
To: Cameron Taylor
Cc: ecos-patches@sources.redhat.com
Subject: Re: Pipe Library


On Fri, Dec 24, 2004 at 11:28:38AM -0700, Cameron Taylor wrote:
> We have developed a pipe implementation which we find useful for
> interprocess communication. It is also the basis for the pty
implementation
> in the following submission.
>
> As seems appropriate for an embedded system, pipes are instantiated
> statically. The same mechanism is used as for other devices: an entry is
> made in the DEVTAB for each pipe the application wishes to use. An
arbitrary
> number of pipes can be created. The application then uses open(), write(),
> read(), select(), etc. to access the pipe. Both read and write selects are
> supported.
>
> One option implemented is a timeout applied to reads and writes set on
> instantiation: FOREVER, NO_WAIT or a timeout in ticks (this is separate
from
> the select timeout).
>
> The pipe can also be purged (pipelib_purge just calls fsync which discards
> any content).
>
> We have inserted the attached files in packages/io/pipe/... and made the
> following entry in ecos.db
>
> package CYGPKG_IO_PIPELIB {
>     alias        { "Pipe Driver Library"}
>     directory    io/pipe
>     script       pipelib.cdl
>     description  "This package contains a device driver for pipes."
> }

I've taken a very brief look, not looking at the sources in
details. These are just the things that jump out at me now. Im sure
there will be other comments when i take a deeper look.

I would drop the reference to lib everythere. The whole of eCos is a
library, so a library inside a library does not make much sense.

I would also remove pipelib_purge. You have the functionaly via
cyg_set_config() so there is no need to duplicate it.

Where possible you should try to use the device driver API, not the
kernel API. I think it should be simple to change this. See

http://ecos.sourceware.org/docs-latest/ref/devapi-api.html

and the pages before. The advantage of using the driver API is that it
always exists. You can configure eCos without the kernel and your code
then fails. If you absolutely must use the KAPI, you need to add a
requires statement to your CDL so that it gets disabled when the
kernel is not present.

The timeouts are interesting, but are not normal for POSIX FIFOs. I
would be tempted to add CDL to control the compilation of this and
have it disabled by default. On the flip side you might want to make
the block on opening the name pipe more Unix like. ie if you open it
for writing it will block until some other thread opens the device for
writing. See

http://www.opengroup.org/onlinepubs/7990989775/xsh/open.html

Some of the copyright statements need cleaning up. Any totally new
files just need copyright eCosCentric Ltd. You cannot keep your own
copyright statements, but please feel free to put a company name etc
in the authors/contributers field.

        Andrew

Attachment: pipe.cdl
Description: Binary data

Attachment: pipe.h
Description: Binary data

Attachment: pipe.c
Description: Binary data