From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from relay00.pair.com ([209.68.5.9]) by bombadil.infradead.org with smtp (Exim 4.68 #1 (Red Hat Linux)) id 1KcjkZ-0002PQ-Hm for linux-mtd@lists.infradead.org; Mon, 08 Sep 2008 16:38:07 +0000 Date: Mon, 08 Sep 2008 09:36:52 -0700 Subject: Re: [PATCH 6/6] [MTD-UTILS] nandwrite: Add Support for Reading from Standard Input From: Grant Erickson To: Ladislav Michl Message-ID: In-Reply-To: <20080908161418.GA7185@michl.2n.cz> Mime-version: 1.0 Content-type: text/plain; charset="US-ASCII" Content-transfer-encoding: 7bit Cc: Tommi Airikka , linux-mtd@lists.infradead.org, Richard Titmuss List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 9/8/08 9:14 AM, Ladislav Michl wrote: > On Sun, Sep 07, 2008 at 09:29:19PM -0700, Grant Erickson wrote: >> Added suppport for reading in band data from standard input based on a >> patch originally generated by Richard Titmuss >> at >> . >> >> Signed-off-by: Grant Erickson >> --- >> >> In addition to the above patch, further discussion of this feature was >> raised by Tommi Airikka at >> . > > Hmm, so long I was defered with implementing this feature, that someone > else did it :-) > > Anyway, let me raise few objections: Fair enough. However, considering that there is sufficient desire for this feature and it has been implemented several times (a number of them off-list and never submitted) let me propose getting this in place and in tree as-is and then evolving and improving it. > - code uses lseek. Obviously, you cannot seek on pipe. True enough. The patch avoids the lseek to determine input file size; however, did not address the 'markbad' recovery lseek case. Short-term proposal: like 'writeoob', disable 'markbad' support when reading from standard input and condition the whole 'markbad' logic on not reading from standard input. > - there is no need to use "invariant placeholder". What about simply > read till file ends? Perhaps Richard can comment on his motivation here. I would suggest that the loop termination condition is not quite that simple since the termination cases are 1) there is no more input data to read (error or EOF) or 2) we have moved outside of the output device bounds. > - there is no need to use special case for reading from stdin... The patch as-is from Richard did for whatever reasons he chose to go that route. Perhaps he can comment on his motivations. However, there may be an opportunity to unify the regular file versus standard input cases along with adding support for '-l,--length=length'. > So what about: > > while (not eof) > read page (and oob if selected) data into buffer What about a short read in which you only get part of a page or a whole page and part of the OOB data? Bail out? Best effort? > next: > if (write buffer fails) > mark bad, select next page, goto next > > Reading from stdin would imply padding. Care to implement that? For the time being, padding must be explicitly requested. > PS: It is remotely possible that one day I do it myself, but I'm > notoriously slow on doing things I do not urgently need. Understood and agreed. For the present, the patch satisfies my cited use case (clearly those of Richard and hopefully those of Tommi) and given the value added, I'd advocate its inclusion and incremental evolution through future patches. Regards, Grant