[PATCH mtd-utils] ubi-tests: io_paral: Fix error handling of update_volume()

[PATCH mtd-utils] ubi-tests: io_paral: Fix error handling of update_volume()

[Martin Lund]

The io_paral test returns success even in case it throws e.g. the
following error message:

[io_paral] update_volume():125: written and read data are different

This patch fixes so that the io_paral application returns a non-zero
error code when an error is detected.
---
 tests/ubi-tests/io_paral.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

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)) {
-- 
2.17.1

Re: [PATCH mtd-utils] ubi-tests: io_paral: Fix error handling of update_volume()

[David Oberhollenzer]

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

Re: [PATCH mtd-utils] ubi-tests: io_paral: Fix error handling of update_volume()

[Martin Lund]

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/