epoll_ctl and const correctness

[nicolas sitbon]

Currently, the prototype of epoll_ctl is :

int epoll_ctl(int epfd, int op, int fd, struct epoll_event *event);

I searched in the man of epoll_ctl and google, and it seems that the
structure pointed to by event isn't modify, valgrind confirms this
behaviour, so am I wrong? or the good prototype is

int epoll_ctl(int epfd, int op, int fd, struct epoll_event const *event);

Thanks by advance!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Davide Libenzi]

On Wed, 25 Mar 2009, nicolas sitbon wrote:

> Currently, the prototype of epoll_ctl is :
>
> int epoll_ctl(int epfd, int op, int fd, struct epoll_event *event);
>
> I searched in the man of epoll_ctl and google, and it seems that the
> structure pointed to by event isn't modify, valgrind confirms this
> behaviour, so am I wrong? or the good prototype is
>
> int epoll_ctl(int epfd, int op, int fd, struct epoll_event const *event);

According to the current ctl operations, yes. But doing that would prevent
other non-const operations to be added later on.

- Davide

[nicolas sitbon]

Ok for "const correctness", but:
do you keep track of the structure event after the call to epoll_ctl or no?
I mean I found some codes that reuse this structure for multiple call
to epoll_ctl but with different file descriptor, is this allowed?

struct epoll_event ev = {.fd = STDIN_FILENO, .data.fd = STDIN_FILENO};
/* <= one structure for 2 calls */
epoll_ctl(epfd, EPOLL_CTL_ADD, STDIN_FILENO, &ev);
ev.fd = STDOUT_FILENO;
ev.data.fd = STDOUT_FILENO;
epoll_ctl(epfd, EPOLL_CTL_ADD, STDOUT_FILENO, &ev);

Other question:
int epoll_create(int size);

can you please, explain me why size is an int and not a size_t or at
least an unsigned integer?

thanks.

2009/3/25 Davide Libenzi <dav...@xmailserver.org>:

[Jeremy Fitzhardinge]

nicolas sitbon wrote:
> valgrind confirms this
> behaviour, so am I wrong?

That doesn't prove very much. Unlike usermode code, Valgrind doesn't
instrument the kernel, so it computes the side-effects of kernel
operations by parsing the syscall stream and simulating the effect.
(That is to say, it strengthens your argument somewhat, but valgrind's
handling of this syscall could be buggy.)

> or the good prototype is
>
> int epoll_ctl(int epfd, int op, int fd, struct epoll_event const *event);
>

Putting "const" first is conventional.

J

[nicolas sitbon]

You don't teach me anything, I know that, the fact is the
documentation is incomplete, so rather saying that, please answer my
questions. For the moment, only the documenation and the prototype of
epoll are buggy.

2009/3/25 Jeremy Fitzhardinge <jer...@goop.org>:

[nicolas sitbon]

Please, can anyone answer me, I need a response.

2009/3/25 nicolas sitbon <nicolas...@gmail.com>:

[nicolas sitbon]

According to this link : http://lse.sourceforge.net/epoll/index.html,
you're are the author of epoll, so please can you answer my questions.
TBA.

2009/3/25 Davide Libenzi <dav...@xmailserver.org>:

[Michael Tokarev]

nicolas sitbon wrote:
> Please, can anyone answer me, I need a response.

> 2009/3/25 nicolas sitbon <nicolas...@gmail.com>:
>> You don't teach me anything, I know that, the fact is the
>> documentation is incomplete, so rather saying that, please answer my
>> questions. For the moment, only the documenation and the prototype of
>> epoll are buggy.

So which response do you want -- the one saying that the documentation
is buggy or or epoll prototype? Or something else?

[]

>>>> or the good prototype is
>>>>
>>>> int epoll_ctl(int epfd, int op, int fd, struct epoll_event const *event);

Why should it be const? There is no guarantee the argument will not be
modified by the kernel. Documentation does not say that. Current prototype
does not say that. If you need such a guarantee, you're free to add another
system call into your kernel, and fix both your documentation and your
prototype to match. What's the deal?

Back from useless rants and to the technical points.

Again: there's no guarantee the `event' argument will not be modified.
Even if kernel CURRENTLY indeed does not modify it, but the interface
does not PROMISE it to be that way for ever.

Why does it not promise that is another question. Just one example:
what, some day, stops us from adding some EPOLL_CTL_GET operation
to RETRIEVE information associated with that filedescriptor in kernel
currently and STORE that info in the structure pointed to by `event'
argument? That way it will not be const anymore.

So.. what's your problem?

/mjt

[nicolas sitbon]

Well, first, thanks for your answer, then, there is a difference
between saying the kernel modify or not the structure and the kernel
keep track of it. Consider this user :
http://www.csplayer.org/2009/02/a-tutorial-example-of-epoll-usage/, he
thinks the kernel doesn't keep track of the event, and I'm sure he is
not the first, so please, at least, be more explicit in the
documentation and again thanks for your answer. And what about size
parameter in epoll_create()? why is it an int and not a size_t?

2009/3/27 Michael Tokarev <m...@tls.msk.ru>:

[nicolas sitbon]

I was looking at libevent of niels provos, and even him, is apparently
doing a mistake :

static int
epoll_add(void *arg, struct event *ev)
{
struct epollop *epollop = arg;
struct epoll_event epev = {0, {0}};

/* ... some code here ... */
if (epoll_ctl(epollop->epfd, op, ev->ev_fd, &epev) == -1)
return (-1);

/* Update events responsible */
if (ev->ev_events & EV_READ)
evep->evread = ev;
if (ev->ev_events & EV_WRITE)
evep->evwrite = ev;

return (0);
}

the structure pointed to by &epev is allocated on the stack, so how
the kernel could keep track of it?

2009/3/27 nicolas sitbon <nicolas...@gmail.com>:

[Davide Libenzi]

On Fri, 27 Mar 2009, nicolas sitbon wrote:

> I was looking at libevent of niels provos, and even him, is apparently
> doing a mistake :
>
> static int
> epoll_add(void *arg, struct event *ev)
> {
> struct epollop *epollop = arg;
> struct epoll_event epev = {0, {0}};
>
> /* ... some code here ... */
> if (epoll_ctl(epollop->epfd, op, ev->ev_fd, &epev) == -1)
> return (-1);
>
> /* Update events responsible */
> if (ev->ev_events & EV_READ)
> evep->evread = ev;
> if (ev->ev_events & EV_WRITE)
> evep->evwrite = ev;
>
> return (0);
> }
>
> the structure pointed to by &epev is allocated on the stack, so how
> the kernel could keep track of it?

Oh, I see, Niels is doing mistakes, whereas you've all figured out.

- Davide

[Michael Tokarev]

nicolas sitbon wrote:
> I was looking at libevent of niels provos, and even him, is apparently
> doing a mistake :
>
> static int
> epoll_add(void *arg, struct event *ev)
> {
> struct epollop *epollop = arg;
> struct epoll_event epev = {0, {0}};
>
> /* ... some code here ... */
> if (epoll_ctl(epollop->epfd, op, ev->ev_fd, &epev) == -1)
> return (-1);
>
> /* Update events responsible */
> if (ev->ev_events & EV_READ)
> evep->evread = ev;
> if (ev->ev_events & EV_WRITE)
> evep->evwrite = ev;
>
> return (0);
> }
>
> the structure pointed to by &epev is allocated on the stack, so how
> the kernel could keep track of it?

I've no idea what are you talking about and what exactly
is your problem. Both the examples (one at cplayer.org and
another above) gives correct usage of epoll. If you don't
understand it, well, I'd suggest reading some documentation
first, maybe in kernel, maybe in glibc, maybe numerous
available on the net. And all the numerous examples too,
which you quote as, for some reason, wrong.

[nicolas sitbon]

Do you read my post? there is a confusion, my question was first : why
the structure isn't const? and you answer me. ok. second, I asked if
the kernel keep track of this structure, that is, it keeps a pointer
on it for doing his job? according to your last answer, no. last
question : why the size type of the first parameter of epoll_create is
int and not size_t? no answer for the moment.

2009/3/27 Michael Tokarev <m...@tls.msk.ru>:

[Michael Tokarev]

nicolas sitbon wrote:
> Do you read my post? there is a confusion, my question was first : why
> the structure isn't const? and you answer me. ok. second, I asked if
> the kernel keep track of this structure, that is, it keeps a pointer
> on it for doing his job? according to your last answer, no. last
> question : why the size type of the first parameter of epoll_create is
> int and not size_t? no answer for the moment.

Why can't you ask all your questions at once?

The answer to your last one is in the manpage. size_t, as
name suggests, is used for sizes of various memory regions,
in bytes. Here, we're dealing with something else, not bytes.

And probably the next question will be of the sort why the argument
is named 'event' and not 'Event' or 'EventPtr', and why there's no
space after the star character... Oh well.

[nicolas sitbon]

is it stupid to ask how a size can be negative? who is stupid? you're
boring with my question, just tell me please how a size can be
negative?

2009/3/28 Michael Tokarev <m...@tls.msk.ru>:

[nicolas sitbon]

read() and poll() are very old functions, a long time ago, int was
nearly the only used type. Today, people uses good programming
practices, mixing information (the size read) and error is a very poor
design and a bad practice, as with read().

2009/3/28 Michael Tokarev <m...@tls.msk.ru>:
> nicolas sitbon wrote:
>>
>> is it stupid to ask how a size can be negative? who is stupid? you're
>> boring with my question, just tell me please how a size can be
>> negative?
>

> Yes I'm bored with your questions.  And am suggesting you to stop
> asking me now.  That's enough.
>
> As of size being negative - how one can read() negative number
> of bytes for example?  How one can poll() waiting for a maximum
> of negative number of milliseconds?
>
> This is my last reply to you.  I have something more useful to do.