I agree. But this was required by Maxim, If I understood him right. You can have a look at the previous versions (v4, v3) for different alternatives. I don't know what to do now. Maxim & Bill: If you can agree on something, I'd be happy to round that up :-).
Christophe On 6 April 2016 at 02:18, Bill Fischofer <bill.fischo...@linaro.org> wrote: > > > On Tue, Apr 5, 2016 at 3:03 AM, Christophe Milard < > christophe.mil...@linaro.org> wrote: > >> Fixes: https://bugs.linaro.org/show_bug.cgi?id=2146 (CID 159395) >> The open system call is directely used to check the presence of the fifo >> and open it at the same time. >> >> Signed-off-by: Christophe Milard <christophe.mil...@linaro.org> >> --- >> since v4: test after loop (Maxim) >> since v3: nb_sec incremented in loop rather then at test time. (Maxim) >> since v2: bug URL added (Anders) >> since v1: changed loop to avoid open() line duplication (Maxim) >> >> platform/linux-generic/test/shmem/shmem_linux.c | 13 ++++++++----- >> 1 file changed, 8 insertions(+), 5 deletions(-) >> >> diff --git a/platform/linux-generic/test/shmem/shmem_linux.c >> b/platform/linux-generic/test/shmem/shmem_linux.c >> index 12266cc..5a9be4d 100644 >> --- a/platform/linux-generic/test/shmem/shmem_linux.c >> +++ b/platform/linux-generic/test/shmem/shmem_linux.c >> @@ -50,6 +50,7 @@ >> >> #define ODP_APP_NAME "shmem_odp" /* name of the odp program, in this dir >> */ >> #define DEVNAME_FMT "odp-%d-%s" /* shm device format: odp-<pid>-<name> >> */ >> +#define MAX_FIFO_WAIT 30 /* Max time waiting for the fifo (sec) >> */ >> >> void test_success(char *fifo_name, int fd, pid_t odp_app) >> { >> @@ -89,7 +90,7 @@ int main(int argc __attribute__((unused)), char *argv[]) >> { >> char prg_name[PATH_MAX]; >> char odp_name[PATH_MAX]; >> - int nb_sec = 0; >> + int nb_sec; >> int size; >> pid_t odp_app; >> char *odp_params = NULL; >> @@ -115,12 +116,14 @@ int main(int argc __attribute__((unused)), char >> *argv[]) >> * Just die if time expire as there is no fifo to communicate >> * through... */ >> sprintf(fifo_name, FIFO_NAME_FMT, odp_app); >> - while (access(fifo_name, W_OK) != 0) { >> + for (nb_sec = 0; nb_sec < MAX_FIFO_WAIT; nb_sec++) { >> + fifo_fd = open(fifo_name, O_WRONLY); >> + if (fifo_fd >= 0) >> + break; >> sleep(1); >> - if (nb_sec++ == 30) >> - exit(1); >> } >> - fifo_fd = open(fifo_name, O_WRONLY); >> + if (nb_sec >= MAX_FIFO_WAIT) >> + exit(1); >> > > Shouldn't the real test here be: > if (fifo_fd < 0) > exit(1); > > That's the actual error condition you're checking for. nb_sec >= > MAX_FIFO_WAIT is an indirect test based on the previous loop running to > completion, however it's error prone if some future maintenance is done > that would result in the loop exiting early. > > >> printf("pipe found\n"); >> >> /* the linux named pipe has now been found, meaning that the >> -- >> 2.1.4 >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp >> > >
_______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
