* [PATCH] nandwrite: Factor out buffer checking code
@ 2016-12-08 4:13 Marek Vasut
2016-12-08 6:35 ` Boris Brezillon
0 siblings, 1 reply; 3+ messages in thread
From: Marek Vasut @ 2016-12-08 4:13 UTC (permalink / raw)
To: linux-mtd; +Cc: Marek Vasut, Boris Brezillon, David Oberhollenzer, Kees Trommel
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>
---
NOTE: This is only compile-tested.
---
nand-utils/nandwrite.c | 42 ++++++++++++++++++++++++++++--------------
1 file changed, 28 insertions(+), 14 deletions(-)
diff --git a/nand-utils/nandwrite.c b/nand-utils/nandwrite.c
index b376869..c5bd1c2 100644
--- a/nand-utils/nandwrite.c
+++ b/nand-utils/nandwrite.c
@@ -231,6 +231,31 @@ static void erase_buffer(void *buffer, size_t size)
memset(buffer, kEraseByte, size);
}
+/* Check whether buffer is filled with character 'pattern' */
+static 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);
+}
+
/*
* Main program
*/
@@ -523,18 +548,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 +558,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) {
--
2.10.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] nandwrite: Factor out buffer checking code
2016-12-08 4:13 [PATCH] nandwrite: Factor out buffer checking code Marek Vasut
@ 2016-12-08 6:35 ` Boris Brezillon
2016-12-09 1:49 ` Marek Vasut
0 siblings, 1 reply; 3+ messages in thread
From: Boris Brezillon @ 2016-12-08 6:35 UTC (permalink / raw)
To: Marek Vasut; +Cc: linux-mtd, David Oberhollenzer, Kees Trommel
Hi Marek,
On Thu, 8 Dec 2016 05:13:42 +0100
Marek Vasut <marex@denx.de> wrote:
> Pull the buffer content checking code into separate function and
> simplify the code invoking it slightly.
Thanks for taking the time to prepare and send this patch.
>
> 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>
> ---
> NOTE: This is only compile-tested.
> ---
> nand-utils/nandwrite.c | 42 ++++++++++++++++++++++++++++--------------
> 1 file changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/nand-utils/nandwrite.c b/nand-utils/nandwrite.c
> index b376869..c5bd1c2 100644
> --- a/nand-utils/nandwrite.c
> +++ b/nand-utils/nandwrite.c
> @@ -231,6 +231,31 @@ static void erase_buffer(void *buffer, size_t size)
> memset(buffer, kEraseByte, size);
> }
>
> +/* Check whether buffer is filled with character 'pattern' */
> +static 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);
> +}
> +
Can we put that in include/common.h as an inline function so that other
tools/libs can re-use it?
I just grepped for the 0xff/0xFF pattern and found several places that
could use this function instead of open-coding it.
Looks good otherwise.
Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> /*
> * Main program
> */
> @@ -523,18 +548,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 +558,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) {
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] nandwrite: Factor out buffer checking code
2016-12-08 6:35 ` Boris Brezillon
@ 2016-12-09 1:49 ` Marek Vasut
0 siblings, 0 replies; 3+ messages in thread
From: Marek Vasut @ 2016-12-09 1:49 UTC (permalink / raw)
To: Boris Brezillon; +Cc: linux-mtd, David Oberhollenzer, Kees Trommel
On 12/08/2016 07:35 AM, Boris Brezillon wrote:
> Hi Marek,
>
> On Thu, 8 Dec 2016 05:13:42 +0100
> Marek Vasut <marex@denx.de> wrote:
>
>> Pull the buffer content checking code into separate function and
>> simplify the code invoking it slightly.
>
> Thanks for taking the time to prepare and send this patch.
>
>>
>> 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>
>> ---
>> NOTE: This is only compile-tested.
>> ---
>> nand-utils/nandwrite.c | 42 ++++++++++++++++++++++++++++--------------
>> 1 file changed, 28 insertions(+), 14 deletions(-)
>>
>> diff --git a/nand-utils/nandwrite.c b/nand-utils/nandwrite.c
>> index b376869..c5bd1c2 100644
>> --- a/nand-utils/nandwrite.c
>> +++ b/nand-utils/nandwrite.c
>> @@ -231,6 +231,31 @@ static void erase_buffer(void *buffer, size_t size)
>> memset(buffer, kEraseByte, size);
>> }
>>
>> +/* Check whether buffer is filled with character 'pattern' */
>> +static 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);
>> +}
>> +
>
> Can we put that in include/common.h as an inline function so that other
> tools/libs can re-use it?
> I just grepped for the 0xff/0xFF pattern and found several places that
> could use this function instead of open-coding it.
>
> Looks good otherwise.
>
> Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>
>
OK, done and V2 out. But it'd be great if someone could actually test it
:-) Kees ?
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-12-09 1:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-08 4:13 [PATCH] nandwrite: Factor out buffer checking code Marek Vasut
2016-12-08 6:35 ` Boris Brezillon
2016-12-09 1:49 ` Marek Vasut
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox