| Message ID | 1448239164-65666-2-git-send-email-champetier.etienne@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On 23/11/2015 01:39, Etienne CHAMPETIER wrote: > spawn_jail(void) produce a compilation error, > so we use spawn_jail() > > Signed-off-by: Etienne CHAMPETIER <champetier.etienne@gmail.com> > --- > jail/jail.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/jail/jail.c b/jail/jail.c > index 56dc9ca..08babde 100644 > --- a/jail/jail.c > +++ b/jail/jail.c > @@ -272,7 +272,7 @@ static int exec_jail() > exit(EXIT_FAILURE); > } > > -static int spawn_jail(void *arg) > +static int spawn_jail() Hi, this is still wrong. also you might want to write a sentence why it is useless. John > { > if (opts.name && sethostname(opts.name, strlen(opts.name))) { > ERROR("failed to sethostname: %s\n", strerror(errno)); > @@ -424,7 +424,7 @@ int main(int argc, char **argv) > if (opts.namespace) { > jail_process.pid = clone(spawn_jail, > child_stack + STACK_SIZE, > - CLONE_NEWUTS | CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | SIGCHLD, argv); > + CLONE_NEWUTS | CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | SIGCHLD, NULL); > } else { > jail_process.pid = fork(); > } >
Hi, Le 23 nov. 2015 08:18, "John Crispin" <blogic@openwrt.org> a écrit : > > > > On 23/11/2015 01:39, Etienne CHAMPETIER wrote: > > spawn_jail(void) produce a compilation error, > > so we use spawn_jail() > > > > Signed-off-by: Etienne CHAMPETIER <champetier.etienne@gmail.com> > > --- > > jail/jail.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/jail/jail.c b/jail/jail.c > > index 56dc9ca..08babde 100644 > > --- a/jail/jail.c > > +++ b/jail/jail.c > > @@ -272,7 +272,7 @@ static int exec_jail() > > exit(EXIT_FAILURE); > > } > > > > -static int spawn_jail(void *arg) > > +static int spawn_jail() > > Hi, > > this is still wrong. also you might want to write a sentence why it is > useless. > Well, we don't use it, and passing arg to not use it really doesn't improve readability. I know it break your code style, but it seems we can't have both here :( Etienne > John > > > { > > if (opts.name && sethostname(opts.name, strlen(opts.name))) { > > ERROR("failed to sethostname: %s\n", strerror(errno)); > > @@ -424,7 +424,7 @@ int main(int argc, char **argv) > > if (opts.namespace) { > > jail_process.pid = clone(spawn_jail, > > child_stack + STACK_SIZE, > > - CLONE_NEWUTS | CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | SIGCHLD, argv); > > + CLONE_NEWUTS | CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | SIGCHLD, NULL); > > } else { > > jail_process.pid = fork(); > > } > >
On 23/11/2015 09:09, Etienne Champetier wrote: > Hi, > > Le 23 nov. 2015 08:18, "John Crispin" <blogic@openwrt.org > <mailto:blogic@openwrt.org>> a écrit : >> >> >> >> On 23/11/2015 01:39, Etienne CHAMPETIER wrote: >> > spawn_jail(void) produce a compilation error, >> > so we use spawn_jail() >> > >> > Signed-off-by: Etienne CHAMPETIER <champetier.etienne@gmail.com > <mailto:champetier.etienne@gmail.com>> >> > --- >> > jail/jail.c | 4 ++-- >> > 1 file changed, 2 insertions(+), 2 deletions(-) >> > >> > diff --git a/jail/jail.c b/jail/jail.c >> > index 56dc9ca..08babde 100644 >> > --- a/jail/jail.c >> > +++ b/jail/jail.c >> > @@ -272,7 +272,7 @@ static int exec_jail() >> > exit(EXIT_FAILURE); >> > } >> > >> > -static int spawn_jail(void *arg) >> > +static int spawn_jail() >> >> Hi, >> >> this is still wrong. also you might want to write a sentence why it is >> useless. >> > > Well, we don't use it, and passing arg to not use it really doesn't > improve readability. > > I know it break your code style, but it seems we can't have both here :( > > Etienne > >> John i wont merge it until there is a (void). it is valid code. try it and you will see that i am right. >> >> > { >> > if (opts.name <http://opts.name> && sethostname(opts.name > <http://opts.name>, strlen(opts.name <http://opts.name>))) { >> > ERROR("failed to sethostname: %s\n", strerror(errno)); >> > @@ -424,7 +424,7 @@ int main(int argc, char **argv) >> > if (opts.namespace) { >> > jail_process.pid = clone(spawn_jail, >> > child_stack + STACK_SIZE, >> > - CLONE_NEWUTS | CLONE_NEWPID | CLONE_NEWNS | > CLONE_NEWIPC | SIGCHLD, argv); >> > + CLONE_NEWUTS | CLONE_NEWPID | CLONE_NEWNS | > CLONE_NEWIPC | SIGCHLD, NULL); >> > } else { >> > jail_process.pid = fork(); >> > } >> > >
2015-11-23 9:11 GMT+01:00 John Crispin <blogic@openwrt.org>: > > > On 23/11/2015 09:09, Etienne Champetier wrote: > > Hi, > > > > Le 23 nov. 2015 08:18, "John Crispin" <blogic@openwrt.org > > <mailto:blogic@openwrt.org>> a écrit : > >> > >> > >> > >> On 23/11/2015 01:39, Etienne CHAMPETIER wrote: > >> > spawn_jail(void) produce a compilation error, > >> > so we use spawn_jail() > >> > > >> > Signed-off-by: Etienne CHAMPETIER <champetier.etienne@gmail.com > > <mailto:champetier.etienne@gmail.com>> > >> > --- > >> > jail/jail.c | 4 ++-- > >> > 1 file changed, 2 insertions(+), 2 deletions(-) > >> > > >> > diff --git a/jail/jail.c b/jail/jail.c > >> > index 56dc9ca..08babde 100644 > >> > --- a/jail/jail.c > >> > +++ b/jail/jail.c > >> > @@ -272,7 +272,7 @@ static int exec_jail() > >> > exit(EXIT_FAILURE); > >> > } > >> > > >> > -static int spawn_jail(void *arg) > >> > +static int spawn_jail() > >> > >> Hi, > >> > >> this is still wrong. also you might want to write a sentence why it is > >> useless. > >> > > > > Well, we don't use it, and passing arg to not use it really doesn't > > improve readability. > > > > I know it break your code style, but it seems we can't have both here :( > > > > Etienne > > > >> John > > > i wont merge it until there is a (void). it is valid code. try it and > you will see that i am right. > > sorry my commit message is bad try to put NULL at the end of the clone() call, and try with spawn_jail() and spawn_jail(void), and you will see i know that spawn_jail(void) is valid code, but then the clone call refuses to compile */home/etienne/procd/jail/jail.c: In function ‘main’:* > */home/etienne/procd/jail/jail.c:381:4: error: passing argument 1 of > ‘clone’ from incompatible pointer type [-Werror]* > * CLONE_NEWUTS | CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | SIGCHLD, > NULL);* > * ^* > *In file included from /usr/include/sched.h:41:0,* > * from /home/etienne/procd/jail/jail.c:26:* > */usr/include/x86_64-linux-gnu/bits/sched.h:81:12: note: expected ‘int > (*)(void *)’ but argument is of type ‘int (*)(void)’* > * extern int clone (int (*__fn) (void *__arg), void *__child_stack,* > * ^* > *cc1: all warnings being treated as errors* > *make[2]: *** [CMakeFiles/ujail.dir/jail/jail.c.o] Error 1* > *make[1]: *** [CMakeFiles/ujail.dir/all] Error 2* > *make: *** [all] Error 2* > > > >> > >> > { > >> > if (opts.name <http://opts.name> && sethostname(opts.name > > <http://opts.name>, strlen(opts.name <http://opts.name>))) { > >> > ERROR("failed to sethostname: %s\n", strerror(errno)); > >> > @@ -424,7 +424,7 @@ int main(int argc, char **argv) > >> > if (opts.namespace) { > >> > jail_process.pid = clone(spawn_jail, > >> > child_stack + STACK_SIZE, > >> > - CLONE_NEWUTS | CLONE_NEWPID | CLONE_NEWNS | > > CLONE_NEWIPC | SIGCHLD, argv); > >> > + CLONE_NEWUTS | CLONE_NEWPID | CLONE_NEWNS | > > CLONE_NEWIPC | SIGCHLD, NULL); > >> > } else { > >> > jail_process.pid = fork(); > >> > } > >> > > > >
Hey Etienne, Etienne Champetier <champetier.etienne@gmail.com> writes: > i know that spawn_jail(void) is valid code, but then the clone call > refuses to compile That's type-safety for you. spawn_jail() is valid code too but it's not type-safe, so AFAICT you're avoiding the errors by letting the compiler ignore them, not by providing a proper solution. :)
Hey, 2015-11-23 12:52 GMT+01:00 Paul Fertser <fercerpav@gmail.com>: > Hey Etienne, > > Etienne Champetier <champetier.etienne@gmail.com> writes: > > i know that spawn_jail(void) is valid code, but then the clone call > > refuses to compile > > That's type-safety for you. spawn_jail() is valid code too but it's not > type-safe, so AFAICT you're avoiding the errors by letting the compiler > ignore them, not by providing a proper solution. :) > clone really want a function like "int fn(void * arg)" Is > *static int spawn_jail(void * _notused)* > ok for you both? Etienne
On 23/11/2015 13:12, Etienne Champetier wrote: > Hey, > > 2015-11-23 12:52 GMT+01:00 Paul Fertser <fercerpav@gmail.com > <mailto:fercerpav@gmail.com>>: > > Hey Etienne, > > Etienne Champetier <champetier.etienne@gmail.com > <mailto:champetier.etienne@gmail.com>> writes: > > i know that spawn_jail(void) is valid code, but then the clone call > > refuses to compile > > That's type-safety for you. spawn_jail() is valid code too but it's not > type-safe, so AFAICT you're avoiding the errors by letting the compiler > ignore them, not by providing a proper solution. :) > > > clone really want a function like "int fn(void * arg)" > > Is > > /static int spawn_jail(void * _notused)/ > > ok for you both? > > Etienne that would be valid code that does not ignore type safety checks.
diff --git a/jail/jail.c b/jail/jail.c index 56dc9ca..08babde 100644 --- a/jail/jail.c +++ b/jail/jail.c @@ -272,7 +272,7 @@ static int exec_jail() exit(EXIT_FAILURE); } -static int spawn_jail(void *arg) +static int spawn_jail() { if (opts.name && sethostname(opts.name, strlen(opts.name))) { ERROR("failed to sethostname: %s\n", strerror(errno)); @@ -424,7 +424,7 @@ int main(int argc, char **argv) if (opts.namespace) { jail_process.pid = clone(spawn_jail, child_stack + STACK_SIZE, - CLONE_NEWUTS | CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | SIGCHLD, argv); + CLONE_NEWUTS | CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | SIGCHLD, NULL); } else { jail_process.pid = fork(); }
spawn_jail(void) produce a compilation error, so we use spawn_jail() Signed-off-by: Etienne CHAMPETIER <champetier.etienne@gmail.com> --- jail/jail.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)