From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mail-wj0-x22c.google.com ([2a00:1450:400c:c01::22c])
 by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux))
 id 1cEUMG-0004me-Fv
 for linux-mtd@lists.infradead.org; Wed, 07 Dec 2016 05:01:33 +0000
Received: by mail-wj0-x22c.google.com with SMTP id tg4so92483726wjb.1
 for <linux-mtd@lists.infradead.org>; Tue, 06 Dec 2016 21:01:10 -0800 (PST)
Subject: Re: Add --skip-all-ffs option to mtd-utils nandwrite
To: David Oberhollenzer <david.oberhollenzer@sigma-star.at>,
 Kees Trommel <ctrommel@aimvalley.nl>, linux-mtd@lists.infradead.org
References: <82cbfdbc-c79b-01b0-bc64-a4259d7a150e@aimvalley.nl>
 <16abefc9-61e8-3b0d-c44c-6d02764ec8e9@sigma-star.at>
 <33437f45-d3e8-17ec-d5b0-0118d50ff6ec@aimvalley.nl>
 <cb18365f-4fe0-9e07-f7cc-6c2bf72018b3@sigma-star.at>
From: Marek Vasut <marek.vasut@gmail.com>
Message-ID: <15c62cdb-38f7-76b8-b3fc-6b3c2766e9df@gmail.com>
Date: Wed, 7 Dec 2016 06:01:08 +0100
MIME-Version: 1.0
In-Reply-To: <cb18365f-4fe0-9e07-f7cc-6c2bf72018b3@sigma-star.at>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
List-Id: Linux MTD discussion mailing list <linux-mtd.lists.infradead.org>
List-Unsubscribe: <http://lists.infradead.org/mailman/options/linux-mtd>,
 <mailto:linux-mtd-request@lists.infradead.org?subject=unsubscribe>
List-Archive: <http://lists.infradead.org/pipermail/linux-mtd/>
List-Post: <mailto:linux-mtd@lists.infradead.org>
List-Help: <mailto:linux-mtd-request@lists.infradead.org?subject=help>
List-Subscribe: <http://lists.infradead.org/mailman/listinfo/linux-mtd>,
 <mailto:linux-mtd-request@lists.infradead.org?subject=subscribe>

On 12/06/2016 02:19 PM, David Oberhollenzer wrote:
> On 12/06/2016 09:29 AM, Kees Trommel wrote:
>> David,
>>
>> Attached a rebased patch file.
>>
>> Kees.
> Applied to mtd-utils.git.

It'd be great to have the patch posted to the list with git send-email ,
otherwise it's not possible to review it :-(

Also, the code doesn't look awesome, see below:

+               if (skipallffs)
+               {
+                       for (ii = 0; ii < mtd.min_io_size; ii +=
sizeof(uint32_t))
+                       {
+                               if (*(uint32_t*)(writebuf + ii) !=
0xffffffff)
+                                       break;

Is this memcmp()-alike function ?

+                       }
+                       if (ii == mtd.min_io_size)
+                               allffs = 1;
+               }
+               if (!allffs) {

This could be simply turned to:

ret = 0;
if (!memcmp(writebuf, writebuf + 1, mtd.min_io_size - 1)) {
    ret = mtd_write(....);
}

And then you don't need to introduce any new vars (which have weird
names like 'ii' ) either.

+                       /* Write out data */
+                       ret = mtd_write(mtd_desc, &mtd, fd, mtdoffset /
mtd.eb_size,
+                                       mtdoffset % mtd.eb_size,
+                                       onlyoob ? NULL : writebuf,
+                                       onlyoob ? 0 : mtd.min_io_size,
+                                       writeoob ? oobbuf : NULL,
+                                       writeoob ? mtd.oob_size : 0,
+                                       write_mode);
+               } else  {
+                       ret = 0;
+               }

-- 
Best regards,
Marek Vasut