On Fri, 4 Mar 2011 08:30:14 -0800 Garrett Cooper <[email protected]> wrote:
> On Fri, Mar 4, 2011 at 8:29 AM, Garrett Cooper <[email protected]> wrote: > > On Fri, Mar 4, 2011 at 8:09 AM, Cristian Greco <[email protected]> wrote: > >> Hi, > >> > >> this should fix a regression in kill05. > >> > >> Why did tst_resm/tst_exit get replaced by perror/exit in 84f181fd? > > > > Because it's a child process and child processes should _not_ use > > libltp for sanity and to avoid introducing determinism, test bugs, and > > s/determinism/non-determinism/ > > > for cluttering up output (remember: you have to communicate to the > > user which process is actually doing the work, they're sharing file > > descriptors potentially, etc). > > The proposed fix doesn't make sense though because do_master_child > > always exits when the flow is executed correctly (i.e. there aren't > > any early return statements from do_master_child, etc. Is it not > > resuming the stack appropriately after it gets signaled? What's the > > call flow exactly for the failure? > > Thanks, > > -Garrett > > Hi Garrett, you're right. In fact, the problem is in safe_getpwnam(), which does not correctly reproduce the behavior of the deprecated my_getpwnam(). Moreover, I'm not noticing the problem on my laptop due to a race condition in kill05, while it shows an inconsistent behavior on the box I'm running tests on (mipsel with glibc, for the sake). Could you please fix safe_getpwnam() with either one of the following patches? --- a/lib/safe_macros.c +++ b/lib/safe_macros.c @@ -98,13 +98,18 @@ struct passwd* safe_getpwnam(const char *file, const int lineno, void (*cleanup_fn)(void), const char *name) { + struct passwd *pwd; struct passwd *rval; - rval = getpwnam(name); - if (rval == NULL) + pwd = getpwnam(name); + if (pwd == NULL) tst_brkm(TBROK|TERRNO, cleanup_fn, "getpwnam failed at %s:%d", file, lineno); + rval = (struct passwd *) malloc(sizeof(struct passwd)); + memset(rval, 0, sizeof(struct passwd)); + memcpy(rval, pwd, sizeof(struct passwd)); + return (rval); } This second patch is a bit more invasive, using getpwnam_r() to correctly retrieve the strings which getpwnam() will store in static buffers (e.g. pw_name in kill05 is 'bin' for both the parent and the child, while their UID is still different). --- a/lib/safe_macros.c +++ b/lib/safe_macros.c @@ -98,9 +98,26 @@ struct passwd* safe_getpwnam(const char *file, const int lineno, void (*cleanup_fn)(void), const char *name) { + struct passwd *pwd; struct passwd *rval; + char *buf; + size_t bufsize; - rval = getpwnam(name); + bufsize = sysconf(_SC_GETPW_R_SIZE_MAX); + if (bufsize == -1) + bufsize = 16384; + + buf = malloc(bufsize); + if (buf == NULL) + tst_brkm(TBROK|TERRNO, cleanup_fn, "malloc failed at %s:%d", + file, lineno); + + pwd = malloc(sizeof(struct passwd)); + if (buf == NULL) + tst_brkm(TBROK|TERRNO, cleanup_fn, "malloc failed at %s:%d", + file, lineno); + + getpwnam_r(name, pwd, buf, bufsize, &rval); if (rval == NULL) tst_brkm(TBROK|TERRNO, cleanup_fn, "getpwnam failed at %s:%d", file, lineno); On the other hand, the race condition in kill05 should be fixed with something like this (sorry, please double-check, I have not so much experience with pthreads): --- a/testcases/kernel/syscalls/kill/kill05.c +++ b/testcases/kernel/syscalls/kill/kill05.c @@ -77,6 +77,7 @@ #include <stdio.h> #include <stdlib.h> #include <unistd.h> +#include <pthread.h> #include "test.h" #include "usctest.h" @@ -101,6 +102,9 @@ extern int getipckey(); #define TEST_SIG SIGKILL +pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; +pthread_cond_t cond = PTHREAD_COND_INITIALIZER; + int main(int ac, char **av) { char *msg; /* message returned from parse_opts */ @@ -169,6 +173,11 @@ void do_master_child(char **av) exit(1); } #else + pthread_mutex_lock(&mutex); + *flag = 1; + pthread_cond_signal(&cond); + pthread_mutex_unlock(&mutex); + do_child(); #endif } @@ -177,10 +186,19 @@ void do_master_child(char **av) exit(1); } + pthread_mutex_lock(&mutex); + while (*flag == 0) + pthread_cond_wait(&cond, &mutex); + pthread_mutex_unlock(&mutex); + TEST(kill(pid1, TEST_SIG)); /* signal the child that we're done */ + pthread_mutex_lock(&mutex); *flag = 1; + pthread_cond_signal(&cond); + pthread_mutex_unlock(&mutex); + if (waitpid(pid1, &status, 0) == -1) { perror("waitpid failed"); @@ -206,15 +224,12 @@ void do_master_child(char **av) void do_child() { - pid_t my_pid; - - my_pid = getpid(); - while (1) { - if (*flag == 1) - exit(0); - else - sleep(1); - } + pthread_mutex_lock(&mutex); + while (*flag == 0) + pthread_cond_wait(&cond, &mutex); + pthread_mutex_unlock(&mutex); + + exit(0); } void setup(void) Thanks, -- Cristian Greco GPG key ID: 0xCF4D32E4
signature.asc
Description: PGP signature
------------------------------------------------------------------------------ What You Don't Know About Data Connectivity CAN Hurt You This paper provides an overview of data connectivity, details its effect on application quality, and explores various alternative solutions. http://p.sf.net/sfu/progress-d2d
_______________________________________________ Ltp-list mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/ltp-list
