diff mbox

[V2] nandwrite: Factor out buffer checking code

Message ID 20161209014741.15609-1-marex@denx.de
State Accepted
Headers show

Commit Message

Marek Vasut Dec. 9, 2016, 1:47 a.m. UTC
Pull the buffer content checking code into separate function and
simplify the code invoking it slightly.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
Cc: Kees Trommel <ctrommel@aimvalley.nl>
---
 include/common.h       | 25 +++++++++++++++++++++++++
 nand-utils/nandwrite.c | 17 +++--------------
 2 files changed, 28 insertions(+), 14 deletions(-)
---
V2: Move the buffer_check_pattern() to include/common.h

Comments

Boris Brezillon Dec. 9, 2016, 7:49 a.m. UTC | #1
On Fri,  9 Dec 2016 02:47:41 +0100
Marek Vasut <marex@denx.de> wrote:

> Pull the buffer content checking code into separate function and
> simplify the code invoking it slightly.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>

Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>

> Cc: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
> Cc: Kees Trommel <ctrommel@aimvalley.nl>
> ---
>  include/common.h       | 25 +++++++++++++++++++++++++
>  nand-utils/nandwrite.c | 17 +++--------------
>  2 files changed, 28 insertions(+), 14 deletions(-)
> ---
> V2: Move the buffer_check_pattern() to include/common.h
> 
> diff --git a/include/common.h b/include/common.h
> index 32b5d23..2812f49 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -181,6 +181,31 @@ static inline int is_power_of_2(unsigned long long n)
>  	return (n != 0 && ((n & (n - 1)) == 0));
>  }
>  
> +/* Check whether buffer is filled with character 'pattern' */
> +static inline int buffer_check_pattern(unsigned char *buffer, size_t size,
> +				       unsigned char pattern)
> +{
> +	/* Invalid input */
> +	if (!buffer || (size == 0))
> +		return 0;
> +
> +	/* No match on first byte */
> +	if (*buffer != pattern)
> +		return 0;
> +
> +	/* First byte matched and buffer is 1 byte long, OK. */
> +	if (size == 1)
> +		return 1;
> +
> +	/*
> +	 * Check buffer longer than 1 byte. We already know that buffer[0]
> +	 * matches the pattern, so the test below only checks whether the
> +	 * buffer[0...size-2] == buffer[1...size-1] , which is a test for
> +	 * whether the buffer is filled with constant value.
> +	 */
> +	return !memcmp(buffer, buffer + 1, size - 1);
> +}
> +
>  /**
>   * simple_strtoX - convert a hex/dec/oct string into a number
>   * @snum: buffer to convert
> diff --git a/nand-utils/nandwrite.c b/nand-utils/nandwrite.c
> index b376869..9602a6e 100644
> --- a/nand-utils/nandwrite.c
> +++ b/nand-utils/nandwrite.c
> @@ -523,18 +523,8 @@ int main(int argc, char * const argv[])
>  			}
>  		}
>  
> -		allffs = 0;
> -		if (skipallffs) 
> -		{
> -			for (ii = 0; ii < mtd.min_io_size; ii += sizeof(uint32_t))
> -			{
> -				if (*(uint32_t*)(writebuf + ii) != 0xffffffff)
> -					break;
> -			}
> -			if (ii == mtd.min_io_size)
> -				allffs = 1;
> -		}
> -		if (!allffs) {
> +		ret = 0;
> +		if (!skipallffs || !buffer_check_pattern(writebuf, mtd.min_io_size, 0xff)) {
>  			/* Write out data */
>  			ret = mtd_write(mtd_desc, &mtd, fd, mtdoffset / mtd.eb_size,
>  					mtdoffset % mtd.eb_size,
> @@ -543,9 +533,8 @@ int main(int argc, char * const argv[])
>  					writeoob ? oobbuf : NULL,
>  					writeoob ? mtd.oob_size : 0,
>  					write_mode);
> -		} else  {
> -			ret = 0;
>  		}
> +
>  		if (ret) {
>  			long long i;
>  			if (errno != EIO) {
David Oberhollenzer Dec. 9, 2016, 8:25 a.m. UTC | #2
On 12/09/2016 08:49 AM, Boris Brezillon wrote:
> On Fri,  9 Dec 2016 02:47:41 +0100
> Marek Vasut <marex@denx.de> wrote:
> 
>> Pull the buffer content checking code into separate function and
>> simplify the code invoking it slightly.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> 
> Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>

Looks good to me too.

On 12/09/2016 02:49 AM, Marek Vasut wrote:
> OK, done and V2 out. But it'd be great if someone could actually test it
> :-) Kees ?
> 
> 

From looking at the code, it looks perfectly reasonable and I don't see
why it shouldn't work.

Anyway, I took the code snippet and ran an exhaustive test on all 32 bit
integers. It appears to work as intended.

Applied to mtd-utils.git.


Thanks,

David
diff mbox

Patch

diff --git a/include/common.h b/include/common.h
index 32b5d23..2812f49 100644
--- a/include/common.h
+++ b/include/common.h
@@ -181,6 +181,31 @@  static inline int is_power_of_2(unsigned long long n)
 	return (n != 0 && ((n & (n - 1)) == 0));
 }
 
+/* Check whether buffer is filled with character 'pattern' */
+static inline int buffer_check_pattern(unsigned char *buffer, size_t size,
+				       unsigned char pattern)
+{
+	/* Invalid input */
+	if (!buffer || (size == 0))
+		return 0;
+
+	/* No match on first byte */
+	if (*buffer != pattern)
+		return 0;
+
+	/* First byte matched and buffer is 1 byte long, OK. */
+	if (size == 1)
+		return 1;
+
+	/*
+	 * Check buffer longer than 1 byte. We already know that buffer[0]
+	 * matches the pattern, so the test below only checks whether the
+	 * buffer[0...size-2] == buffer[1...size-1] , which is a test for
+	 * whether the buffer is filled with constant value.
+	 */
+	return !memcmp(buffer, buffer + 1, size - 1);
+}
+
 /**
  * simple_strtoX - convert a hex/dec/oct string into a number
  * @snum: buffer to convert
diff --git a/nand-utils/nandwrite.c b/nand-utils/nandwrite.c
index b376869..9602a6e 100644
--- a/nand-utils/nandwrite.c
+++ b/nand-utils/nandwrite.c
@@ -523,18 +523,8 @@  int main(int argc, char * const argv[])
 			}
 		}
 
-		allffs = 0;
-		if (skipallffs) 
-		{
-			for (ii = 0; ii < mtd.min_io_size; ii += sizeof(uint32_t))
-			{
-				if (*(uint32_t*)(writebuf + ii) != 0xffffffff)
-					break;
-			}
-			if (ii == mtd.min_io_size)
-				allffs = 1;
-		}
-		if (!allffs) {
+		ret = 0;
+		if (!skipallffs || !buffer_check_pattern(writebuf, mtd.min_io_size, 0xff)) {
 			/* Write out data */
 			ret = mtd_write(mtd_desc, &mtd, fd, mtdoffset / mtd.eb_size,
 					mtdoffset % mtd.eb_size,
@@ -543,9 +533,8 @@  int main(int argc, char * const argv[])
 					writeoob ? oobbuf : NULL,
 					writeoob ? mtd.oob_size : 0,
 					write_mode);
-		} else  {
-			ret = 0;
 		}
+
 		if (ret) {
 			long long i;
 			if (errno != EIO) {