[Xenomai-help] pipe fun

[Xenomai-help] pipe fun

[Ignacio García Pérez]

Hi,

This has to be something really simple but I'm hitting a wall. I have a
rt module that creates a pipe and periodically writes some data to it.
In the following program, the read fails with "No space left on device":


int main (void) {

    int r, fd; u_char c;

    fd = open("/proc/xenomai/registry/pipes/rt2event", O_RDWR);
    if (fd < 0) {
        fprintf(stderr, "ERROR opening pipe: %s\n", strerror(errno));
        return -1;
    }

    for (;;) {
        r = read(fd, &c, sizeof(c));
        if (r == 0) { fprintf(stderr, "ERROR reading (returned 0)\n");
break; }
        if (r < 0) { fprintf(stderr, "ERROR reading: %s\n",
strerror(errno)); break; }
        fprintf(stderr, "%02hX ", c);
    }

    close(fd);

    return 0;
}


However, the following program works fine!!!


int main (void) {

    int r; u_char c;

    FILE *f;

    f = fopen("/proc/xenomai/registry/pipes/rt2event", "r+b");
    if (f == NULL) {
        fprintf(stderr, "ERROR opening pipe: %s\n", strerror(errno));
        return -1;
    }

    for (;;) {
        r = fread(&c, sizeof(c), 1, f);
        if (r == 0) { fprintf(stderr, "ERROR reading (returned 0)\n");
break; }
        if (r < 0) { fprintf(stderr, "ERROR reading: %s\n",
strerror(errno)); break; }
        fprintf(stderr, "%02hX ", c);
    }

    fclose(f);

    return 0;
}



Any clues?

By the way, I noticed a "cat /proc/xenomai/registry/pipes/rt2event |
xxd" won't work, perhaps due to the same reason?

Re: [Xenomai-help] pipe fun

[Dmitry Adamushko]

[-- Attachment #1: Type: text/plain, Size: 3436 bytes --]



xenomai-help-bounces@domain.hid wrote on 21.11.2005 11:11:52:

>
> This has to be something really simple but I'm hitting a wall. I have a
> rt module that creates a pipe and periodically writes some data to it.
> In the following program, the read fails with "No space left on device":

The reason is as follows:

from rt_pipe_send() docs

 * This service writes a complete message to be received from the
 * associated special device. rt_pipe_send() always preserves message
 * boundaries, which means that all data sent through a single call of
 * this service will be gathered in a single read(2) operation from
 * the special device.

Data is written as a size-bounded message so a reader must provide a
big-enough buffer for the message to be read as a whole. It's possible to
send data as a stream (rt_pipe_stream()) but not possible (well, should it
be so?) to read it as a stream (actually, it's possible but as a side
effect - that's why fread() works).

[code]

nucleus::pipe.c::xnpipe_read()

...
        ret = (ssize_t)xnpipe_m_size(mh); /* Cannot be zero */
        state->ionrd -= ret;

        xnlock_put_irqrestore(&nklock,s);

        if (ret <= count)     <---- Is a provided buffer big enough to host
the whole message?
            {
            if (__copy_to_user(buf,xnpipe_m_data(mh),ret))
                ret = -EFAULT;
            }
        else
            /* Return buffer is too small - message is lost. */
            ret = -ENOSPC;    <---- That's the error you are getting.
...

[/code]


Why does fread() work? It just uses an additional buffering (by default)
and provides a big enough internal buffer for read(). So the whole message
is read into that buffer upon the first fread() call from your side and a
few subsequent fread() calls just return data from that internal buffer. So
it looks like data being read as a stream but that's only a consequence of
additional buffering being made by glibc::fread().

>
>
> int main (void) {
>
>     int r, fd; u_char c;
>
>     fd = open("/proc/xenomai/registry/pipes/rt2event", O_RDWR);
>     if (fd < 0) {
>         fprintf(stderr, "ERROR opening pipe: %s\n", strerror(errno));
>         return -1;
>     }
>
>     for (;;) {
>         r = read(fd, &c, sizeof(c));
>         if (r == 0) { fprintf(stderr, "ERROR reading (returned 0)\n");
> break; }
>         if (r < 0) { fprintf(stderr, "ERROR reading: %s\n",
> strerror(errno)); break; }
>         fprintf(stderr, "%02hX ", c);
>     }
>
>     close(fd);
>
>     return 0;
> }
>
>
> However, the following program works fine!!!
>
>
> int main (void) {
>
>     int r; u_char c;
>
>     FILE *f;
>
>     f = fopen("/proc/xenomai/registry/pipes/rt2event", "r+b");
>     if (f == NULL) {
>         fprintf(stderr, "ERROR opening pipe: %s\n", strerror(errno));
>         return -1;
>     }
>
>     for (;;) {
>         r = fread(&c, sizeof(c), 1, f);
>         if (r == 0) { fprintf(stderr, "ERROR reading (returned 0)\n");
> break; }
>         if (r < 0) { fprintf(stderr, "ERROR reading: %s\n",
> strerror(errno)); break; }
>         fprintf(stderr, "%02hX ", c);
>     }
>
>     fclose(f);
>
>     return 0;
> }
>
>
>
> Any clues?
>
> By the way, I noticed a "cat /proc/xenomai/registry/pipes/rt2event |
> xxd" won't work, perhaps due to the same reason?
>

---
Best regards,
Dmitry

[-- Attachment #2: Type: text/html, Size: 5290 bytes --]

Re: [Xenomai-help] pipe fun

[Ignacio García Pérez]

Dmitry Adamushko wrote:

>            /* Return buffer is too small - message is lost. */
>            ret = -ENOSPC; <---- That's the error you are getting.
>
Maybe "No space left on device" is a bit misleading. Wouldn't ENOMEM fit
better? (or event EINVAL)

Re: [Xenomai-help] pipe fun

[Dmitry Adamushko]

[-- Attachment #1: Type: text/plain, Size: 862 bytes --]



Ignacio García Pérez <iggarpe@domain.hid> wrote on 21.11.2005 12:24:57:

> Dmitry Adamushko wrote:
>
> >            /* Return buffer is too small - message is lost. */
> >            ret = -ENOSPC; <---- That's the error you are getting.
> >
> Maybe "No space left on device" is a bit misleading. Wouldn't ENOMEM fit
> better? (or event EINVAL)
>

Actually, looking now at the linux sources, -ENOSPC is used for describing
the same reason (buffer is not big enough) in some places; although there
is, at least, one more value -ENOBUFS that's used for the same reason.

#define ENOSPC          28      /* No space left on device */
#define ENOBUFS         105     /* No buffer space available */

So it looks like both may be used in the same context but, I guess, a
definition of -ENOBUFS is less confusing.


---
Best regards,
Dmitry

[-- Attachment #2: Type: text/html, Size: 1232 bytes --]

Re: [Xenomai-help] pipe fun

[Philippe Gerum]

Dmitry Adamushko wrote:
> Ignacio García Pérez <iggarpe@domain.hid> wrote on 21.11.2005 12:24:57:
> 
>  > Dmitry Adamushko wrote:
>  >
>  > >            /* Return buffer is too small - message is lost. */
>  > >            ret = -ENOSPC; <---- That's the error you are getting.
>  > >
>  > Maybe "No space left on device" is a bit misleading. Wouldn't ENOMEM fit
>  > better? (or event EINVAL)
>  >
> 
> Actually, looking now at the linux sources, -ENOSPC is used for 
> describing the same reason (buffer is not big enough) in some places; 
> although there is, at least, one more value -ENOBUFS that's used for the 
> same reason.
> 
> #define ENOSPC          28      /* No space left on device */
> #define ENOBUFS         105     /* No buffer space available */
> 
> So it looks like both may be used in the same context but, I guess, a 
> definition of -ENOBUFS is less confusing.
> 
> 

Let's fix this.

> ---
> Best regards,
> Dmitry
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Xenomai-help mailing list
> Xenomai-help@domain.hid
> https://mail.gna.org/listinfo/xenomai-help


-- 

Philippe.

Re: [Xenomai-help] pipe fun

[Dmitry Adamushko]

[-- Attachment #1.1: Type: text/plain, Size: 1623 bytes --]



Philippe Gerum <rpm@xenomai.org> wrote on 21.11.2005 13:36:41:

> Dmitry Adamushko wrote:
> > Ignacio García Pérez <iggarpe@domain.hid> wrote on 21.11.2005 12:24:57:
> >
> >  > Dmitry Adamushko wrote:
> >  >
> >  > >            /* Return buffer is too small - message is lost. */
> >  > >            ret = -ENOSPC; <---- That's the error you are getting.
> >  > >
> >  > Maybe "No space left on device" is a bit misleading. Wouldn't ENOMEM
fit
> >  > better? (or event EINVAL)
> >  >
> >
> > Actually, looking now at the linux sources, -ENOSPC is used for
> > describing the same reason (buffer is not big enough) in some places;
> > although there is, at least, one more value -ENOBUFS that's used for
the
> > same reason.
> >
> > #define ENOSPC          28      /* No space left on device */
> > #define ENOBUFS         105     /* No buffer space available */
> >
> > So it looks like both may be used in the same context but, I guess, a
> > definition of -ENOBUFS is less confusing.
> >
> >
>
> Let's fix this.
>

Enclosed.

One more thing about xnpipe_read() though.


...
            }
        else
            /* Return buffer is too small - message is lost. */
            ret = -ENOBUFS;

So the message is lost in such a case. We may easily avoid that by making a
proper check of the size when we are still in the locked section and
putting the message back into the queue (prependq() of course). I think
that would be more sane.


> --
>
> Philippe.

---
Best regards,
Dmitry

(See attached file: pipe-ENOBUFS.patch)(See attached file: ChangeLog.patch)

[-- Attachment #1.2: Type: text/html, Size: 2497 bytes --]

[-- Attachment #2: pipe-ENOBUFS.patch --]
[-- Type: application/octet-stream, Size: 321 bytes --]

--- pipe.c-old	2005-11-21 14:56:25.000000000 +0100
+++ pipe.c	2005-11-21 14:56:59.000000000 +0100
@@ -749,7 +749,7 @@
 	    }
 	else
 	    /* Return buffer is too small - message is lost. */
-	    ret = -ENOSPC;
+	    ret = -ENOBUFS;
 
 	if (handler != NULL)
 	    ret = handler(xnminor_from_state(state),mh,ret,cookie);

[-- Attachment #3: ChangeLog.patch --]
[-- Type: application/octet-stream, Size: 559 bytes --]

--- ChangeLog-old	2005-11-21 15:04:40.000000000 +0100
+++ ChangeLog	2005-11-21 15:04:27.000000000 +0100
@@ -1,4 +1,11 @@
 
+2005-11-21  Dmitry Adamushko <dmitry.adamushko@gmail.com>
+
+	* nucleus/pipe.c (xnpipe_send): Return -ENOBUFS instead of -ENOSPC
+	when a given buffer is not big enough for the entire message to be
+	written in. perror() gives a, hopefully, more-precise and less-confusing
+	description for the former one.
+
 2005-11-18  Heikki Lindholm <holindho@cs.helsinki.fi>
 
 	* include/asm-powerpc/system.h, ksrc/arch/powerpc/switch_64.S: Add

Re: [Xenomai-help] pipe fun

[Philippe Gerum]

Dmitry Adamushko wrote:
> Philippe Gerum <rpm@xenomai.org> wrote on 21.11.2005 13:36:41:
> 
>  > Dmitry Adamushko wrote:
>  > > Ignacio García Pérez <iggarpe@domain.hid> wrote on 21.11.2005 12:24:57:
>  > >
>  > >  > Dmitry Adamushko wrote:
>  > >  >
>  > >  > >            /* Return buffer is too small - message is lost. */
>  > >  > >            ret = -ENOSPC; <---- That's the error you are getting.
>  > >  > >
>  > >  > Maybe "No space left on device" is a bit misleading. Wouldn't 
> ENOMEM fit
>  > >  > better? (or event EINVAL)
>  > >  >
>  > >
>  > > Actually, looking now at the linux sources, -ENOSPC is used for
>  > > describing the same reason (buffer is not big enough) in some places;
>  > > although there is, at least, one more value -ENOBUFS that's used 
> for the
>  > > same reason.
>  > >
>  > > #define ENOSPC          28      /* No space left on device */
>  > > #define ENOBUFS         105     /* No buffer space available */
>  > >
>  > > So it looks like both may be used in the same context but, I guess, a
>  > > definition of -ENOBUFS is less confusing.
>  > >
>  > >
>  >
>  > Let's fix this.
>  >
> 
> Enclosed.
> 

Applied, thanks.

> One more thing about xnpipe_read() though.
> 
> 
> ...
>            }
>        else
>            /* Return buffer is too small - message is lost. */
>            ret = -ENOBUFS;
> 
> So the message is lost in such a case. We may easily avoid that by 
> making a proper check of the size when we are still in the locked 
> section and putting the message back into the queue (prependq() of 
> course). I think that would be more sane.
>

Problem is that if nobody ends up reading the message because the 
application always passes invalid args, internal heap memory may be 
exhausted due to unread messages.

> 
>  > --
>  >
>  > Philippe.
> 
> ---
> Best regards,
> Dmitry
> 
> /(See attached file: pipe-ENOBUFS.patch)//(See attached file: 
> ChangeLog.patch)/
> 


-- 

Philippe.