Message ID | 20180828083113.18198-1-malu@gomspace.com |
---|---|
State | Superseded |
Headers | show |
Series | [mtd-utils] ubi-tests: io_paral: Fix error handling of update_volume() | expand |
Hi, generally, your approach seams reasonable. However there is one small issue with your patch: On 08/28/2018 10:31 AM, Martin Lund wrote: > @@ -302,7 +302,14 @@ int main(int argc, char * const argv[]) > } > > for (i = 0; i < THREADS_NUM; i++) > - pthread_join(threads[i], NULL); > + { > + pthread_join(threads[i], (void **) &ret); > + if (ret != 0) > + error = true; > + } > + "ret" happens to be an integer, you take a pointer to it and cast it to a pointer to a void*, which receives the result from the thread function. There are platforms where sizeof(void*) != sizeof(int) Minor nit pick: the '{' should be on the same line as the for, similar to the rest of the code. Thanks, David
You are right - good catch. While my patch is a common solution it does not apply to all platforms. I'll rework the patch. Thanks. Martin On Tue, Sep 4, 2018 at 9:15 AM David Oberhollenzer <david.oberhollenzer@sigma-star.at> wrote: > > Hi, > > generally, your approach seams reasonable. However there is one small issue with > your patch: > > On 08/28/2018 10:31 AM, Martin Lund wrote: > > @@ -302,7 +302,14 @@ int main(int argc, char * const argv[]) > > } > > > > for (i = 0; i < THREADS_NUM; i++) > > - pthread_join(threads[i], NULL); > > + { > > + pthread_join(threads[i], (void **) &ret); > > + if (ret != 0) > > + error = true; > > + } > > + > > "ret" happens to be an integer, you take a pointer to it and cast it to a pointer > to a void*, which receives the result from the thread function. > > There are platforms where sizeof(void*) != sizeof(int) > > Minor nit pick: the '{' should be on the same line as the for, similar to the rest > of the code. > > Thanks, > > David > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
diff --git a/tests/ubi-tests/io_paral.c b/tests/ubi-tests/io_paral.c index b2b462e..1b6fee3 100644 --- a/tests/ubi-tests/io_paral.c +++ b/tests/ubi-tests/io_paral.c @@ -149,22 +149,22 @@ static void *update_thread(void *ptr) if (ret) { failed("ubi_rmvol"); errorm("cannot remove volume %d", vol_id); - return NULL; + return (void *) -1; } ret = ubi_mkvol(libubi, node, &reqests[vol_id]); if (ret) { failed("ubi_mkvol"); errorm("cannot create volume %d", vol_id); - return NULL; + return (void *) -1; } } ret = update_volume(vol_id, bytes); - if (ret) - return NULL; + if (ret != 0) + return (void *) -1; } - return NULL; + return (void *) 0; } static void *write_thread(void *ptr) @@ -179,7 +179,7 @@ static void *write_thread(void *ptr) if (fd == -1) { failed("open"); errorm("cannot open \"%s\"\n", vol_node); - return NULL; + return (void *) -1; } ret = ubi_set_property(fd, UBI_VOL_PROP_DIRECT_WRITE, 1); @@ -228,12 +228,12 @@ static void *write_thread(void *ptr) } close(fd); - return NULL; + return (void *) 0; } int main(int argc, char * const argv[]) { - int i, ret; + int i, ret, error=false; pthread_t threads[THREADS_NUM]; if (initial_check(argc, argv)) @@ -302,7 +302,14 @@ int main(int argc, char * const argv[]) } for (i = 0; i < THREADS_NUM; i++) - pthread_join(threads[i], NULL); + { + pthread_join(threads[i], (void **) &ret); + if (ret != 0) + error = true; + } + + if (error) + goto remove; for (i = 0; i <= THREADS_NUM; i++) { if (ubi_rmvol(libubi, node, i)) {