| Message ID | 20230930000516.4063681-1-edliaw@google.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | getpgid01: On Android, pgid(1) is 0 instead of 1 | expand |
Hi Edward, > Android's init does not call setpgid(0, 0) so it does not have pgid=1. > 0 is functionally equivalent, since pgid 0 means the pgid is the same as > the process pid. > Signed-off-by: Edward Liaw <edliaw@google.com> > --- > testcases/kernel/syscalls/getpgid/getpgid01.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > diff --git a/testcases/kernel/syscalls/getpgid/getpgid01.c b/testcases/kernel/syscalls/getpgid/getpgid01.c > index 479fe5dcb..8640f2c93 100644 > --- a/testcases/kernel/syscalls/getpgid/getpgid01.c > +++ b/testcases/kernel/syscalls/getpgid/getpgid01.c > @@ -37,7 +37,7 @@ static void run(void) > TST_EXP_EQ_LI(TST_RET, pgid); > TST_EXP_PID(getpgid(1)); > - TST_EXP_EQ_LI(TST_RET, 1); > + TST_EXP_EXPR(TST_RET == 1 || TST_RET == 0, "getpgid(1) == 1 or 0"); Although I don't prefer using often libc specific code, here I'd use it: #ifndef __ANDROID__ TST_EXP_EQ_LI(TST_RET, 0); #else TST_EXP_EQ_LI(TST_RET, 1); #endif Because your code would loosen testing for other libc. Cyril, Li, WDYT? Kind regards, Petr
Hi Petr, On Fri, Sep 29, 2023 at 11:54 PM Petr Vorel <pvorel@suse.cz> wrote: > > Hi Edward, > > > Android's init does not call setpgid(0, 0) so it does not have pgid=1. > > 0 is functionally equivalent, since pgid 0 means the pgid is the same as > > the process pid. > > > Signed-off-by: Edward Liaw <edliaw@google.com> > > --- > > testcases/kernel/syscalls/getpgid/getpgid01.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > diff --git a/testcases/kernel/syscalls/getpgid/getpgid01.c b/testcases/kernel/syscalls/getpgid/getpgid01.c > > index 479fe5dcb..8640f2c93 100644 > > --- a/testcases/kernel/syscalls/getpgid/getpgid01.c > > +++ b/testcases/kernel/syscalls/getpgid/getpgid01.c > > @@ -37,7 +37,7 @@ static void run(void) > > TST_EXP_EQ_LI(TST_RET, pgid); > > > TST_EXP_PID(getpgid(1)); > > - TST_EXP_EQ_LI(TST_RET, 1); > > + TST_EXP_EXPR(TST_RET == 1 || TST_RET == 0, "getpgid(1) == 1 or 0"); > Although I don't prefer using often libc specific code, here I'd use it: > > #ifndef __ANDROID__ > TST_EXP_EQ_LI(TST_RET, 0); > #else > TST_EXP_EQ_LI(TST_RET, 1); > #endif > > Because your code would loosen testing for other libc. I'm fine with that. I tried looking up whether pgid of init should always be 1 but wasn't able to find a good answer, so I defer to your / the team's judgement here. Will send a v2. > Cyril, Li, WDYT? > > Kind regards, > Petr
Hi! > > diff --git a/testcases/kernel/syscalls/getpgid/getpgid01.c b/testcases/kernel/syscalls/getpgid/getpgid01.c > > index 479fe5dcb..8640f2c93 100644 > > --- a/testcases/kernel/syscalls/getpgid/getpgid01.c > > +++ b/testcases/kernel/syscalls/getpgid/getpgid01.c > > @@ -37,7 +37,7 @@ static void run(void) > > TST_EXP_EQ_LI(TST_RET, pgid); > > > TST_EXP_PID(getpgid(1)); > > - TST_EXP_EQ_LI(TST_RET, 1); > > + TST_EXP_EXPR(TST_RET == 1 || TST_RET == 0, "getpgid(1) == 1 or 0"); > Although I don't prefer using often libc specific code, here I'd use it: > > #ifndef __ANDROID__ > TST_EXP_EQ_LI(TST_RET, 0); > #else > TST_EXP_EQ_LI(TST_RET, 1); > #endif > > Because your code would loosen testing for other libc. > Cyril, Li, WDYT? @Peter what about the case where we bypass all init and boot with init=/bin/sh or init=ltx? I suppose that we will have pgrp set to 0 as well. What about parsing /proc/1/stat and checking that the value there is the same as reported here, that should work regardless of what pid is the init.
Hi, ... > > #ifndef __ANDROID__ > > TST_EXP_EQ_LI(TST_RET, 0); > > #else > > TST_EXP_EQ_LI(TST_RET, 1); > > #endif > > Because your code would loosen testing for other libc. > > Cyril, Li, WDYT? > @Peter what about the case where we bypass all init and boot with > init=/bin/sh or init=ltx? I suppose that we will have pgrp set to 0 > as well. Ah, so v1 version was better, right? > What about parsing /proc/1/stat and checking that the value there is the > same as reported here, that should work regardless of what pid is the > init. That sounds best to me. I'm sorry Edward, could you please send v3? Kind regards, Petr
diff --git a/testcases/kernel/syscalls/getpgid/getpgid01.c b/testcases/kernel/syscalls/getpgid/getpgid01.c index 479fe5dcb..8640f2c93 100644 --- a/testcases/kernel/syscalls/getpgid/getpgid01.c +++ b/testcases/kernel/syscalls/getpgid/getpgid01.c @@ -37,7 +37,7 @@ static void run(void) TST_EXP_EQ_LI(TST_RET, pgid); TST_EXP_PID(getpgid(1)); - TST_EXP_EQ_LI(TST_RET, 1); + TST_EXP_EXPR(TST_RET == 1 || TST_RET == 0, "getpgid(1) == 1 or 0"); } tst_reap_children();
Android's init does not call setpgid(0, 0) so it does not have pgid=1. 0 is functionally equivalent, since pgid 0 means the pgid is the same as the process pid. Signed-off-by: Edward Liaw <edliaw@google.com> --- testcases/kernel/syscalls/getpgid/getpgid01.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)