[LTP] [PATCH] lib/safe_macros.c: fix safe_write() and safe_pwrite()

[LTP] [PATCH] lib/safe_macros.c: fix safe_write() and safe_pwrite()

[Guangwen Feng]

The len_strict is ineffective in byte length verification
because of the improper if-condition.

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 lib/safe_macros.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/safe_macros.c b/lib/safe_macros.c
index a3d997c..173d72f 100644
--- a/lib/safe_macros.c
+++ b/lib/safe_macros.c
@@ -414,7 +414,7 @@ ssize_t safe_write(const char *file, const int lineno, void (cleanup_fn) (void),
 	ssize_t rval;
 
 	rval = write(fildes, buf, nbyte);
-	if ((len_strict == 0 && rval == -1) || (size_t)rval != nbyte) {
+	if (rval == -1 || (len_strict && (size_t)rval != nbyte)) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
 			 "%s:%d: write(%d,%p,%zu) failed",
 		         file, lineno, fildes, buf, rval);
@@ -430,7 +430,7 @@ ssize_t safe_pwrite(const char *file, const int lineno,
 	ssize_t rval;
 
 	rval = pwrite(fildes, buf, nbyte, offset);
-	if ((len_strict == 0 && rval == -1) || (size_t)rval != nbyte) {
+	if (rval == -1 || (len_strict && (size_t)rval != nbyte)) {
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
 			 "%s:%d: pwrite(%d,%p,%zu,%ld) failed",
 			 file, lineno, fildes, buf, rval, offset);
-- 
1.8.4.2


------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] lib/safe_macros.c: fix safe_write() and safe_pwrite()

[Jan Stancek]

----- Original Message -----
> From: "Guangwen Feng" <fenggw-fnst@cn.fujitsu.com>
> To: ltp-list@lists.sourceforge.net
> Sent: Tuesday, 14 July, 2015 8:47:03 AM
> Subject: [LTP] [PATCH] lib/safe_macros.c: fix safe_write() and safe_pwrite()
> 
> The len_strict is ineffective in byte length verification
> because of the improper if-condition.
> 
> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>

Good catch, pushed.

Regards,
Jan

> ---
>  lib/safe_macros.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/safe_macros.c b/lib/safe_macros.c
> index a3d997c..173d72f 100644
> --- a/lib/safe_macros.c
> +++ b/lib/safe_macros.c
> @@ -414,7 +414,7 @@ ssize_t safe_write(const char *file, const int lineno,
> void (cleanup_fn) (void),
>  	ssize_t rval;
>  
>  	rval = write(fildes, buf, nbyte);
> -	if ((len_strict == 0 && rval == -1) || (size_t)rval != nbyte) {
> +	if (rval == -1 || (len_strict && (size_t)rval != nbyte)) {
>  		tst_brkm(TBROK | TERRNO, cleanup_fn,
>  			 "%s:%d: write(%d,%p,%zu) failed",
>  		         file, lineno, fildes, buf, rval);
> @@ -430,7 +430,7 @@ ssize_t safe_pwrite(const char *file, const int lineno,
>  	ssize_t rval;
>  
>  	rval = pwrite(fildes, buf, nbyte, offset);
> -	if ((len_strict == 0 && rval == -1) || (size_t)rval != nbyte) {
> +	if (rval == -1 || (len_strict && (size_t)rval != nbyte)) {
>  		tst_brkm(TBROK | TERRNO, cleanup_fn,
>  			 "%s:%d: pwrite(%d,%p,%zu,%ld) failed",
>  			 file, lineno, fildes, buf, rval, offset);
> --
> 1.8.4.2
> 
> 
> ------------------------------------------------------------------------------
> Don't Limit Your Business. Reach for the Cloud.
> GigeNET's Cloud Solutions provide you with the tools and support that
> you need to offload your IT needs and focus on growing your business.
> Configured For All Businesses. Start Your Cloud Today.
> https://www.gigenetcloud.com/
> _______________________________________________
> Ltp-list mailing list
> Ltp-list@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ltp-list
> 

------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] lib/safe_macros.c: fix safe_write() and safe_pwrite()

[Cyril Hrubis]

Hi!
> The len_strict is ineffective in byte length verification
> because of the improper if-condition.
> 
> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
> ---
>  lib/safe_macros.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/safe_macros.c b/lib/safe_macros.c
> index a3d997c..173d72f 100644
> --- a/lib/safe_macros.c
> +++ b/lib/safe_macros.c
> @@ -414,7 +414,7 @@ ssize_t safe_write(const char *file, const int lineno, void (cleanup_fn) (void),
>  	ssize_t rval;
>  
>  	rval = write(fildes, buf, nbyte);
> -	if ((len_strict == 0 && rval == -1) || (size_t)rval != nbyte) {
> +	if (rval == -1 || (len_strict && (size_t)rval != nbyte)) {
>  		tst_brkm(TBROK | TERRNO, cleanup_fn,
>  			 "%s:%d: write(%d,%p,%zu) failed",
>  		         file, lineno, fildes, buf, rval);

Ok, this was wrong because with len_strict == 1 and rval == -1 we will
cast -1 to (size_t) which will yield 2^64 - 1. In practice this was
working fine since nobody would call write with such large buffer...

Newertless it's better as it is now, but commit message could have been
better since as far as I can see the len_strict flag was working fine.

Because when len_strict == 1 the result was computed as:

if ((0 && whatever) || rval != nbyte) -> if (0 || rval != nbyte) -> if (rval != nbyte)

which worked fine for all practical purposes. Or am I missing something?

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] lib/safe_macros.c: fix safe_write() and safe_pwrite()

[Cyril Hrubis]

Hi!
> > Ok, this was wrong because with len_strict == 1 and rval == -1 we will
> > cast -1 to (size_t) which will yield 2^64 - 1. In practice this was
> > working fine since nobody would call write with such large buffer...
> > 
> > Newertless it's better as it is now, but commit message could have been
> > better since as far as I can see the len_strict flag was working fine.
> > 
> > Because when len_strict == 1 the result was computed as:
> > 
> > if ((0 && whatever) || rval != nbyte) -> if (0 || rval != nbyte) -> if (rval
> > != nbyte)
> > 
> > which worked fine for all practical purposes. Or am I missing something?
> 
> Also, when len_strict == 0 and less bytes were written than requested,
> it would report TBROK. With len_strict == 0 it should TBROK only when
> rval is -1.

Ah, that is what I missed. The commit message should said that the lengh
was checked in all cases...

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list